diff --git a/docs/installation.md b/docs/installation.md index a9f722f..6d3717b 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -184,13 +184,23 @@ 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.* @@ -198,6 +208,7 @@ promotionTargetBlockList: promotionTargetAllowList: - env/prod/.* - env/(dev|lab)/.* +disableArgoCDDiff: true ``` ## GitHub API Limit diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go index 8d1b1e9..8a76954 100644 --- a/internal/pkg/argocd/argocd.go +++ b/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 diff --git a/internal/pkg/configuration/config.go b/internal/pkg/configuration/config.go index 0d2bbc8..0ea7066 100644 --- a/internal/pkg/configuration/config.go +++ b/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 { diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go index e49758a..fa5f3d5 100644 --- a/internal/pkg/githubapi/github.go +++ b/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: diff --git a/internal/pkg/githubapi/promotion.go b/internal/pkg/githubapi/promotion.go index 84f1e19..561a9be 100644 --- a/internal/pkg/githubapi/promotion.go +++ b/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)