Allow selectively redacting diff content (#19)

* Add a new component-level "disableDiff" configuration key
* Read the configuration file before diff
* Pass the the config the relevant function
* Replace diff content with a relevant message
* Document new configuration key


Co-authored-by: Yazdan Mohammadi <[email protected]>

---------

Co-authored-by: Yazdan Mohammadi <[email protected]>

docs/installation.md

@@ -184,20 +184,31 @@ toggleCommitStatus:
 This optional in-component configuration file allows overriding the general promotion configuration for a specific component.
 File location is `COMPONENT_PATH/telefonistka.yaml` (no leading dot in file name), so it could be:
 `workspace/reloader/telefonistka.yaml` or `env/prod/us-central1/c2/wf-kube-proxy-metrics-proxy/telefonistka.yaml`
-it includes only two optional configuration keys, `promotionTargetBlockList` and `promotionTargetAllowList`.
-Both are matched against the target component path using Golang regex engine.
+it includes these  optional configuration keys: `promotionTargetBlockList`,  `promotionTargetAllowList` and `disableArgoCDDiff`
+`promotionTargetBlockList` and `promotionTargetAllowList`  are matched against the target component path using Golang regex engine.
 
 If a target path matches an entry in `promotionTargetBlockList` it will not be promoted(regardless of `promotionTargetAllowList`).
 
 If  `promotionTargetAllowList` exist(non empty), only target paths that matches it will be promoted to(but the previous statement about `promotionTargetBlockList` still applies).
 
+`disableArgoCDDiff` can be used to ensure no sensitive information **stored outside `kind:Secret` objects** is persisted to PR comments, this can happen if secrets are injected as part of the ArgoCD manifest templating stage and are stored outside `kind:Secret` objects and/or referenced by hashing function in annotations to trigger restarts. And while both use cases can (and should!) be avoided we choose to provide a workaround to prevent this issues from blocking Telefonistka implementation.
+
+ArgoCD API redact all `kind:Secret` object content automatically so under "normal" usage this is not an issue.
+
+Telefonistka will still display changed objects, just without the content:
+
+![image](https://github.com/user-attachments/assets/f8ebc390-6051-4640-982e-6b768975dcfc)
+
+Example:
+
 ```yaml
 promotionTargetBlockList:
   - env/staging/europe-west4/c1.*
   - env/prod/us-central1/c3/
 promotionTargetAllowList:
   - env/prod/.*
   - env/(dev|lab)/.*
+disableArgoCDDiff: true
 ```
 
 ## GitHub API Limit

internal/pkg/argocd/argocd.go

@@ -53,7 +53,7 @@ type DiffResult struct {
 
 // Mostly copied from  https://github.com/argoproj/argo-cd/blob/4f6a8dce80f0accef7ed3b5510e178a6b398b331/cmd/argocd/commands/app.go#L1255C6-L1338
 // But instead of printing the diff to stdout, we return it as a string in a struct so we can format it in a nice PR comment.
-func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj *argoappv1.AppProject, resources *application.ManagedResourcesResponse, argoSettings *settings.Settings, diffOptions *DifferenceOption) (foundDiffs bool, diffElements []DiffElement, err error) {
+func generateArgocdAppDiff(ctx context.Context, keepDiffData bool, app *argoappv1.Application, proj *argoappv1.AppProject, resources *application.ManagedResourcesResponse, argoSettings *settings.Settings, diffOptions *DifferenceOption) (foundDiffs bool, diffElements []DiffElement, err error) {
 	liveObjs, err := cmdutil.LiveObjects(resources.Items)
 	if err != nil {
 		return false, nil, fmt.Errorf("Failed to get live objects: %w", err)
@@ -126,7 +126,11 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
 				foundDiffs = true
 			}
 
-			diffElement.Diff, err = diffLiveVsTargetObject(live, target)
+			if keepDiffData {
+				diffElement.Diff, err = diffLiveVsTargetObject(live, target)
+			} else {
+				diffElement.Diff = "✂️ ✂️  Redacted ✂️ ✂️ \nUnset component-level configuration key `disableArgoCDDiff` to see diff content."
+			}
 			if err != nil {
 				return false, nil, fmt.Errorf("Failed to diff live objects: %w", err)
 			}
@@ -299,7 +303,7 @@ func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision st
 	return err
 }
 
-func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranch string, repo string, appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, argoSettings *settings.Settings, useSHALabelForArgoDicovery bool) (componentDiffResult DiffResult) {
+func generateDiffOfAComponent(ctx context.Context, commentDiff bool, componentPath string, prBranch string, repo string, appClient application.ApplicationServiceClient, projClient projectpkg.ProjectServiceClient, argoSettings *settings.Settings, useSHALabelForArgoDicovery bool) (componentDiffResult DiffResult) {
 	componentDiffResult.ComponentPath = componentPath
 
 	// Find ArgoCD application by the path SHA1 label selector and repo name
@@ -374,16 +378,14 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
 		return componentDiffResult
 	}
 
-	componentDiffResult.HasDiff, componentDiffResult.DiffElements, err = generateArgocdAppDiff(ctx, app, detailedProject.Project, resources, argoSettings, diffOption)
-	if err != nil {
-		componentDiffResult.DiffError = err
-	}
+	log.Debugf("Generating diff for component %s", componentPath)
+	componentDiffResult.HasDiff, componentDiffResult.DiffElements, componentDiffResult.DiffError = generateArgocdAppDiff(ctx, commentDiff, app, detailedProject.Project, resources, argoSettings, diffOption)
 
 	return componentDiffResult
 }
 
 // GenerateDiffOfChangedComponents generates diff of changed components
-func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []string, prBranch string, repo string, useSHALabelForArgoDicovery bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) {
+func GenerateDiffOfChangedComponents(ctx context.Context, componentsToDiff map[string]bool, prBranch string, repo string, useSHALabelForArgoDicovery bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) {
 	hasComponentDiff = false
 	hasComponentDiffErrors = false
 	// env var should be centralized
@@ -399,9 +401,8 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st
 		return false, true, nil, err
 	}
 
-	log.Debugf("Checking ArgoCD diff for components: %v", componentPathList)
-	for _, componentPath := range componentPathList {
-		currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, ac.app, ac.project, argoSettings, useSHALabelForArgoDicovery)
+	for componentPath, shouldIDiff := range componentsToDiff {
+		currentDiffResult := generateDiffOfAComponent(ctx, shouldIDiff, componentPath, prBranch, repo, ac.app, ac.project, argoSettings, useSHALabelForArgoDicovery)
 		if currentDiffResult.DiffError != nil {
 			log.Errorf("Error generating diff for component %s: %v", componentPath, currentDiffResult.DiffError)
 			hasComponentDiffErrors = true

internal/pkg/configuration/config.go

@@ -12,6 +12,7 @@ type WebhookEndpointRegex struct {
 type ComponentConfig struct {
 	PromotionTargetAllowList []string `yaml:"promotionTargetAllowList"`
 	PromotionTargetBlockList []string `yaml:"promotionTargetBlockList"`
+	DisableArgoCDDiff        bool     `yaml:"disableArgoCDDiff"`
 }
 
 type Condition struct {

internal/pkg/githubapi/github.go

@@ -121,7 +121,24 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr
 				ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err)
 			}
 
-			hasComponentDiff, hasComponentDiffErrors, diffOfChangedComponents, err := argocd.GenerateDiffOfChangedComponents(ctx, componentPathList, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
+			// Building a map component's path and a boolean value that indicates if we should diff it not.
+			// I'm avoiding doing this in the ArgoCD package to avoid circular dependencies and keep package scope clean
+			componentsToDiff := map[string]bool{}
+			for _, componentPath := range componentPathList {
+				c, err := getComponentConfig(ghPrClientDetails, componentPath, ghPrClientDetails.Ref)
+				if err != nil {
+					prHandleError = fmt.Errorf("Failed to get component config(%s):  err=%s\n", componentPath, err)
+					ghPrClientDetails.PrLogger.Error(prHandleError)
+				} else {
+					if c.DisableArgoCDDiff {
+						componentsToDiff[componentPath] = false
+						ghPrClientDetails.PrLogger.Debugf("ArgoCD diff disabled for %s\n", componentPath)
+					} else {
+						componentsToDiff[componentPath] = true
+					}
+				}
+			}
+			hasComponentDiff, hasComponentDiffErrors, diffOfChangedComponents, err := argocd.GenerateDiffOfChangedComponents(ctx, componentsToDiff, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery)
 			if err != nil {
 				prHandleError = err
 				ghPrClientDetails.PrLogger.Errorf("Failed to get ArgoCD diff information: err=%s\n", err)
@@ -304,7 +321,6 @@ func handleEvent(eventPayloadInterface interface{}, mainGhClientCache *lru.Cache
 			PrSHA:        *eventPayload.PullRequest.Head.SHA,
 		}
 
-		log.Debugf("=== Ref is %s\n", ghPrClientDetails.Ref)
 		HandlePREvent(eventPayload, ghPrClientDetails, mainGithubClientPair, approverGithubClientPair, ctx)
 
 	case *github.IssueCommentEvent:

internal/pkg/githubapi/promotion.go

@@ -102,7 +102,7 @@ func getComponentConfig(ghPrClientDetails GhPrClientDetails, componentPath strin
 		return nil, err
 	} else if resp.StatusCode == 404 {
 		ghPrClientDetails.PrLogger.Debugf("No in-component config in %s", componentPath)
-		return nil, nil
+		return &cfg.ComponentConfig{}, nil
 	}
 	componentConfigFileContentString, _ := componentConfigFileContent.GetContent()
 	err = yaml.Unmarshal([]byte(componentConfigFileContentString), componentConfig)