Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow setting argoCD revision to PR git Branch #16

Merged
merged 34 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
1a0b2dc
Allow setting argoCD revision to PR git SHA
Oded-B Jun 28, 2024
d22f30a
Address linter issues
Oded-B Jun 28, 2024
26ad53a
Linter issue
Oded-B Jun 28, 2024
0a550ec
debug defer issue
Oded-B Jun 28, 2024
63325cd
Adress "diffOfChangedComponents is an unexported field of struct" issue
Oded-B Jun 28, 2024
f9b06af
Use comemnt sender login to check in event was triggered by Telefonistka
Oded-B Jun 28, 2024
82225a3
Use the correct field in the event payload
Oded-B Jun 28, 2024
b2ed2bc
fix some whitespace issue
Oded-B Jul 1, 2024
e4cbcad
debug some panic
Oded-B Jul 1, 2024
0c0d06f
Only check for checkbox event on **updates** on comment the bot created.
Oded-B Jul 1, 2024
8cb4c1a
On PR merge - set apps TargetRef to HEAD
Oded-B Jul 1, 2024
ea57c0e
Linter issues
Oded-B Jul 1, 2024
c54fbac
Change method used to apply changes to ArgoCD apps
Oded-B Jul 1, 2024
e20f5b5
Debug " application destination can't have both name and server defin…
Oded-B Jul 1, 2024
1e28e50
Use Patch method to be more surgical.
Oded-B Jul 1, 2024
eb33944
Remove old method for App update and debug message
Oded-B Jul 2, 2024
80f3443
Build patch from a structured object
Oded-B Jul 2, 2024
c235871
Only allow sync for Open PRs
Oded-B Jul 2, 2024
385a31b
Add Namespace to app patching, to ensure telefonistka doesn't try to
Oded-B Jul 2, 2024
469270c
Add debug log line
Oded-B Jul 2, 2024
9db40f0
Explicitly set the NS
Oded-B Jul 2, 2024
779df8f
revert to old version using json tring
Oded-B Jul 2, 2024
b69e031
Add debug statements
Oded-B Jul 2, 2024
1f3ee4b
Add more debug statements
Oded-B Jul 3, 2024
4d88126
Use PR Branch HEAD instead of commit
Oded-B Jul 3, 2024
c72229e
Use an anonymous struct to create the patch
Oded-B Jul 4, 2024
e587299
Document new config key
Oded-B Jul 4, 2024
c93a0c0
Apply suggestions from code review
Oded-B Jul 11, 2024
6a131d3
Use "%w" for warapped errors
Oded-B Jul 11, 2024
47e002b
Use table tests
Oded-B Jul 11, 2024
efbe16e
Address PR comments
Oded-B Jul 11, 2024
80601dd
Group ArgoCD clients in a struct per Arthur's PR comment
Oded-B Jul 12, 2024
0a7eee2
Move the Regex pattern in analyzeCommentUpdateCheckBox (while keeping
Oded-B Jul 12, 2024
3834ddd
Improve log message to address PR comment
Oded-B Jul 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ Configuration keys:
|`commentArgocdDiffonPR`| Uses ArgoCD API to calculate expected changes to k8s state and comment the resulting "diff" as comment in the PR. Requires ARGOCD_* environment variables, see below. |
|`autoMergeNoDiffPRs`| if true, Telefonistka will **merge** promotion PRs that are not expected to change the target clusters. Requires `commentArgocdDiffonPR` and possibly `autoApprovePromotionPrs`(depending on repo branch protection rules)|
|`useSHALabelForArgoDicovery`| The default method for discovering relevant ArgoCD applications (for a PR) relies on fetching all applications in the repo and checking the `argocd.argoproj.io/manifest-generate-paths` **annotation**, this might cause a performance issue on a repo with a large number of ArgoCD applications. The alternative is to add SHA1 of the application path as a **label** and rely on ArgoCD server-side filtering, label name is `telefonistka.io/component-path-sha1`.|
|`allowSyncArgoCDAppfromBranchPathRegex`| This controls which component(=ArgoCD apps) are allowed to be "applied" from a PR branch, by setting the ArgoCD application `Target Revision` to PR branch.|
<!-- markdownlint-enable MD033 -->

Example:
Expand Down Expand Up @@ -173,6 +174,7 @@ dryRunMode: true
autoApprovePromotionPrs: true
commentArgocdDiffonPR: true
autoMergeNoDiffPRs: true
allowSyncArgoCDAppfromBranchPathRegex: '^workspace/.*$'
toggleCommitStatus:
override-terrafrom-pipeline: "github-action-terraform"
```
Expand Down
130 changes: 92 additions & 38 deletions internal/pkg/argocd/argocd.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,18 @@ import (
argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1"
argodiff "github.com/argoproj/argo-cd/v2/util/argo/diff"
"github.com/argoproj/argo-cd/v2/util/argo/normalizers"
argoio "github.com/argoproj/argo-cd/v2/util/io"
"github.com/argoproj/gitops-engine/pkg/sync/hook"
"github.com/google/go-cmp/cmp"
log "github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
)

type argoCdClients struct {
app application.ApplicationServiceClient
project projectpkg.ProjectServiceClient
setting settings.SettingsServiceClient
}

// DiffElement struct to store diff element details, this represents a single k8s object
type DiffElement struct {
ObjectGroup string
Expand All @@ -51,25 +56,25 @@ type DiffResult struct {
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) {
liveObjs, err := cmdutil.LiveObjects(resources.Items)
if err != nil {
return false, nil, fmt.Errorf("Failed to get live objects: %v", err)
return false, nil, fmt.Errorf("Failed to get live objects: %w", err)
}

items := make([]objKeyLiveTarget, 0)
var unstructureds []*unstructured.Unstructured
for _, mfst := range diffOptions.res.Manifests {
obj, err := argoappv1.UnmarshalToUnstructured(mfst)
if err != nil {
return false, nil, fmt.Errorf("Failed to unmarshal manifest: %v", err)
return false, nil, fmt.Errorf("Failed to unmarshal manifest: %w", err)
}
unstructureds = append(unstructureds, obj)
}
groupedObjs, err := groupObjsByKey(unstructureds, liveObjs, app.Spec.Destination.Namespace)
if err != nil {
return false, nil, fmt.Errorf("Failed to group objects by key: %v", err)
return false, nil, fmt.Errorf("Failed to group objects by key: %w", err)
}
items, err = groupObjsForDiff(resources, groupedObjs, items, argoSettings, app.InstanceName(argoSettings.ControllerNamespace), app.Spec.Destination.Namespace)
if err != nil {
return false, nil, fmt.Errorf("Failed to group objects for diff: %v", err)
return false, nil, fmt.Errorf("Failed to group objects for diff: %w", err)
}

for _, item := range items {
Expand All @@ -91,11 +96,11 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
WithNoCache().
Build()
if err != nil {
return false, nil, fmt.Errorf("Failed to build diff config: %v", err)
return false, nil, fmt.Errorf("Failed to build diff config: %w", err)
}
diffRes, err := argodiff.StateDiff(item.live, item.target, diffConfig)
if err != nil {
return false, nil, fmt.Errorf("Failed to diff objects: %v", err)
return false, nil, fmt.Errorf("Failed to diff objects: %w", err)
}

if diffRes.Modified || item.target == nil || item.live == nil {
Expand All @@ -111,7 +116,7 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj
live = item.live
err = json.Unmarshal(diffRes.PredictedLive, target)
if err != nil {
return false, nil, fmt.Errorf("Failed to unmarshal predicted live object: %v", err)
return false, nil, fmt.Errorf("Failed to unmarshal predicted live object: %w", err)
}
} else {
live = item.live
Expand All @@ -123,7 +128,7 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj

diffElement.Diff, err = diffLiveVsTargetObject(live, target)
if err != nil {
return false, nil, fmt.Errorf("Failed to diff live objects: %v", err)
return false, nil, fmt.Errorf("Failed to diff live objects: %w", err)
}
}
diffElements = append(diffElements, diffElement)
Expand All @@ -144,7 +149,7 @@ func getEnv(key, fallback string) string {
return fallback
}

func createArgoCdClient() (apiclient.Client, error) {
func createArgoCdClients() (ac argoCdClients, err error) {
plaintext, _ := strconv.ParseBool(getEnv("ARGOCD_PLAINTEXT", "false"))
insecure, _ := strconv.ParseBool(getEnv("ARGOCD_INSECURE", "false"))

Expand All @@ -155,11 +160,26 @@ func createArgoCdClient() (apiclient.Client, error) {
Insecure: insecure,
}

clientset, err := apiclient.NewClient(opts)
client, err := apiclient.NewClient(opts)
if err != nil {
return ac, fmt.Errorf("Error creating ArgoCD API client: %w", err)
}

_, ac.app, err = client.NewApplicationClient()
if err != nil {
return ac, fmt.Errorf("Error creating ArgoCD app client: %w", err)
}

_, ac.project, err = client.NewProjectClient()
if err != nil {
return ac, fmt.Errorf("Error creating ArgoCD project client: %w", err)
}

_, ac.setting, err = client.NewSettingsClient()
if err != nil {
return nil, fmt.Errorf("Error creating ArgoCD API client: %v", err)
return ac, fmt.Errorf("Error creating ArgoCD settings client: %w", err)
}
return clientset, nil
return
}

// findArgocdAppBySHA1Label finds an ArgoCD application by the SHA1 label of the component path it's supposed to avoid performance issues with the "manifest-generate-paths" annotation method which requires pulling all ArgoCD applications(!) on every PR event.
Expand All @@ -178,7 +198,7 @@ func findArgocdAppBySHA1Label(ctx context.Context, componentPath string, repo st
}
foundApps, err := appClient.List(ctx, &appLabelQuery)
if err != nil {
return nil, fmt.Errorf("Error listing ArgoCD applications: %v", err)
return nil, fmt.Errorf("Error listing ArgoCD applications: %w", err)
}
if len(foundApps.Items) == 0 {
return nil, fmt.Errorf("No ArgoCD application found for component path sha1 %s(repo %s), used this label selector: %s", componentPathSha1, repo, labelSelector)
Expand Down Expand Up @@ -231,6 +251,54 @@ func findArgocdAppByManifestPathAnnotation(ctx context.Context, componentPath st
return nil, fmt.Errorf("No ArgoCD application found with manifest-generate-paths annotation that matches %s(looked at repo %s, checked %v apps) ", componentPath, repo, len(allRepoApps.Items))
}

func SetArgoCDAppRevision(ctx context.Context, componentPath string, revision string, repo string, useSHALabelForArgoDicovery bool) error {
var foundApp *argoappv1.Application
var err error
ac, err := createArgoCdClients()
if err != nil {
return fmt.Errorf("Error creating ArgoCD clients: %w", err)
}
if useSHALabelForArgoDicovery {
foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, ac.app)
} else {
foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, ac.app)
}
if err != nil {
return fmt.Errorf("error finding ArgoCD application for component path %s: %w", componentPath, err)
}
if foundApp.Spec.Source.TargetRevision == revision {
log.Infof("App %s already has revision %s", foundApp.Name, revision)
return nil
}

patchObject := struct {
Spec struct {
Source struct {
TargetRevision string `json:"targetRevision"`
} `json:"source"`
} `json:"spec"`
}{}
Oded-B marked this conversation as resolved.
Show resolved Hide resolved
patchObject.Spec.Source.TargetRevision = revision
patchJson, _ := json.Marshal(patchObject)
patch := string(patchJson)
log.Debugf("Patching app %s/%s with: %s", foundApp.Namespace, foundApp.Name, patch)

patchType := "merge"
Oded-B marked this conversation as resolved.
Show resolved Hide resolved
_, err = ac.app.Patch(ctx, &application.ApplicationPatchRequest{
Name: &foundApp.Name,
AppNamespace: &foundApp.Namespace,
PatchType: &patchType,
Patch: &patch,
})
if err != nil {
return fmt.Errorf("revision patching failed: %w", err)
} else {
log.Infof("ArgoCD App %s revision set to %s", foundApp.Name, revision)
}

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) {
componentDiffResult.ComponentPath = componentPath

Expand Down Expand Up @@ -266,6 +334,12 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc
log.Debugf("Got ArgoCD app %s", app.Name)
componentDiffResult.ArgoCdAppName = app.Name
componentDiffResult.ArgoCdAppURL = fmt.Sprintf("%s/applications/%s", argoSettings.URL, app.Name)

if app.Spec.Source.TargetRevision == prBranch {
componentDiffResult.DiffError = fmt.Errorf("App %s already has revision %s as Source Target Revision, skipping diff calculation", app.Name, prBranch)
return componentDiffResult
}

resources, err := appClient.ManagedResources(ctx, &application.ResourcesQuery{ApplicationName: &app.Name, AppNamespace: &app.Namespace})
if err != nil {
componentDiffResult.DiffError = err
Expand Down Expand Up @@ -313,41 +387,21 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st
hasComponentDiff = false
hasComponentDiffErrors = false
// env var should be centralized
client, err := createArgoCdClient()
ac, err := createArgoCdClients()
if err != nil {
log.Errorf("Error creating ArgoCD client: %v", err)
log.Errorf("Error creating ArgoCD clients: %v", err)
return false, true, nil, err
}

conn, appClient, err := client.NewApplicationClient()
if err != nil {
log.Errorf("Error creating ArgoCD app client: %v", err)
return false, true, nil, err
}
defer argoio.Close(conn)

conn, projClient, err := client.NewProjectClient()
if err != nil {
log.Errorf("Error creating ArgoCD project client: %v", err)
return false, true, nil, err
}
defer argoio.Close(conn)

conn, settingClient, err := client.NewSettingsClient()
if err != nil {
log.Errorf("Error creating ArgoCD settings client: %v", err)
return false, true, nil, err
}
defer argoio.Close(conn)
argoSettings, err := settingClient.Get(ctx, &settings.SettingsQuery{})
argoSettings, err := ac.setting.Get(ctx, &settings.SettingsQuery{})
if err != nil {
log.Errorf("Error getting ArgoCD settings: %v", err)
return false, true, nil, err
}

log.Debugf("Checking ArgoCD diff for components: %v", componentPathList)
for _, componentPath := range componentPathList {
currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appClient, projClient, argoSettings, useSHALabelForArgoDicovery)
currentDiffResult := generateDiffOfAComponent(ctx, 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
Expand Down
19 changes: 10 additions & 9 deletions internal/pkg/configuration/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,16 @@ type Config struct {
PromotionPaths []PromotionPath `yaml:"promotionPaths"`

// Generic configuration
PromtionPrLables []string `yaml:"promtionPRlables"`
DryRunMode bool `yaml:"dryRunMode"`
AutoApprovePromotionPrs bool `yaml:"autoApprovePromotionPrs"`
ToggleCommitStatus map[string]string `yaml:"toggleCommitStatus"`
WebhookEndpointRegexs []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"`
WhProxtSkipTLSVerifyUpstream bool `yaml:"whProxtSkipTLSVerifyUpstream"`
CommentArgocdDiffonPR bool `yaml:"commentArgocdDiffonPR"`
AutoMergeNoDiffPRs bool `yaml:"autoMergeNoDiffPRs"`
UseSHALabelForArgoDicovery bool `yaml:"useSHALabelForArgoDicovery"`
PromtionPrLables []string `yaml:"promtionPRlables"`
DryRunMode bool `yaml:"dryRunMode"`
AutoApprovePromotionPrs bool `yaml:"autoApprovePromotionPrs"`
ToggleCommitStatus map[string]string `yaml:"toggleCommitStatus"`
WebhookEndpointRegexs []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"`
WhProxtSkipTLSVerifyUpstream bool `yaml:"whProxtSkipTLSVerifyUpstream"`
CommentArgocdDiffonPR bool `yaml:"commentArgocdDiffonPR"`
AutoMergeNoDiffPRs bool `yaml:"autoMergeNoDiffPRs"`
AllowSyncArgoCDAppfromBranchPathRegex string `yaml:"allowSyncArgoCDAppfromBranchPathRegex"`
UseSHALabelForArgoDicovery bool `yaml:"useSHALabelForArgoDicovery"`
}

func ParseConfigFromYaml(y string) (*Config, error) {
Expand Down
Loading
Loading