feat: add flag to include .git

[clstb]

Hello,

some tools like git-crypt require to receive the repository metadata as well and are commonly used to encrypt sensitive data alongside k8s configurations, so I think this should be supported.

[zapier-sre-bot]

Mergecat's Review

Click to read mergecats review!

😼 Mergecat review of pkg/config/config.go

@@ -27,6 +27,7 @@ type ServerConfig struct {
 	ArgoCDRepositoryEndpoint string `mapstructure:"argocd-repository-endpoint"`
 	ArgoCDRepositoryInsecure bool   `mapstructure:"argocd-repository-insecure"`
 	ArgoCDSendFullRepository bool   `mapstructure:"argocd-send-full-repository"`
+	ArgoCDIncludeDotGit      bool   `mapstructure:"argocd-include-dot-git"`
 	KubernetesConfig         string `mapstructure:"kubernetes-config"`
 	KubernetesType           string `mapstructure:"kubernetes-type"`
 	KubernetesClusterID      string `mapstructure:"kubernetes-clusterid"`

Feedback & Suggestions:

1. Documentation: Ensure that the new field ArgoCDIncludeDotGit is documented properly. This includes updating any relevant documentation files or comments within the code to explain what this field does and how it should be used. 📚

2. Validation: Consider adding validation logic to ensure that the value of ArgoCDIncludeDotGit is set correctly. This could be done in the NewWithViper function or elsewhere in the code where configuration is validated. This helps prevent misconfigurations. ✅

3. Testing: If there are unit tests or integration tests for the configuration, ensure that tests are added or updated to cover this new field. This will help maintain the reliability of the codebase. 🧪

---

😼 Mergecat review of pkg/argo_client/manifests.go

@@ -153,7 +153,13 @@ func (a *ArgoClient) generateManifests(ctx context.Context, app v1alpha1.Applica
 	}
 
 	log.Info().Msg("compressing files")
-	f, filesWritten, checksum, err := tgzstream.CompressFiles(packageDir, []string{"*"}, []string{".git"})
+
+	exclude := []string{}
+	if !a.cfg.ArgoCDIncludeDotGit {
+		exclude = append(exclude, ".git")
+	}
+
+	f, filesWritten, checksum, err := tgzstream.CompressFiles(packageDir, []string{"*"}, exclude)
 	if err != nil {
 		return nil, fmt.Errorf("failed to compress files: %w", err)
 	}

Feedback & Suggestions:

1. Security Consideration: Ensure that the ArgoCDIncludeDotGit configuration is securely managed. Including .git directories in compressed files can expose sensitive information such as commit history and configuration details. Make sure this option is only enabled when absolutely necessary and that the implications are well understood.

2. Code Clarity: The logic for determining the exclude list is clear, but consider adding a comment explaining the purpose of excluding .git directories. This will help future maintainers understand the rationale behind this decision.

3. Performance: The change introduces a conditional check for ArgoCDIncludeDotGit. Ensure that this configuration is not frequently toggled at runtime, as it could lead to inconsistent behavior or performance issues if not handled properly.

4. Testing: Ensure that there are adequate tests covering both scenarios (including and excluding .git directories) to verify that the behavior is as expected in both cases.

Overall, the change is well-structured and improves flexibility by allowing the inclusion or exclusion of .git directories based on configuration. Just be cautious about the security implications. 🛡️

---

---

Dependency Review

Click to read mergecats review!

No suggestions found