diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c011f0ce..65b3644d 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -34,6 +34,7 @@ jobs: with: go-version: 1.21 - uses: actions/checkout@v4 + - run: make get-deps - name: golangci-lint uses: golangci/golangci-lint-action@v6 with: diff --git a/Makefile b/Makefile index 60fb3c13..26adb331 100644 --- a/Makefile +++ b/Makefile @@ -12,6 +12,7 @@ VENDOR_DIR = vendor get-deps: $(VENDOR_DIR) $(VENDOR_DIR): + go generate $$(go list ./internal/pkg/mocks/...) GO111MODULE=on go mod vendor .PHONY: build @@ -24,5 +25,5 @@ clean: .PHONY: test test: $(VENDOR_DIR) - go test -v -timeout 30s . + go test -v -timeout 30s ./... diff --git a/docs/installation.md b/docs/installation.md index 050eb35d..0ccdc27d 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -119,6 +119,7 @@ Configuration keys: |`toggleCommitStatus`| Map of strings, allow (non-repo-admin) users to change the [Github commit status](https://docs.github.com/en/rest/commits/statuses) state(from failure to success and back). This can be used to continue promotion of a change that doesn't pass repo checks. the keys are strings commented in the PRs, values are [Github commit status context](https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#create-a-commit-status) to be overridden| |`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`.| Example: diff --git a/go.mod b/go.mod index 6907ddd1..ac697386 100644 --- a/go.mod +++ b/go.mod @@ -6,8 +6,8 @@ toolchain go1.22.1 require ( github.com/alexliesenfeld/health v0.8.0 - github.com/argoproj/argo-cd/v2 v2.11.0-rc1 - github.com/argoproj/gitops-engine v0.7.1-0.20240411122334-1ade3a199867 + github.com/argoproj/argo-cd/v2 v2.11.2 + github.com/argoproj/gitops-engine v0.7.1-0.20240416142647-fbecbb86e412 github.com/bradleyfalzon/ghinstallation/v2 v2.10.0 github.com/cenkalti/backoff/v4 v4.2.1 github.com/go-test/deep v1.1.0 @@ -22,7 +22,7 @@ require ( github.com/shurcooL/githubv4 v0.0.0-20240120211514-18a1ae0e79dc github.com/sirupsen/logrus v1.9.3 github.com/spf13/cobra v1.8.0 - golang.org/x/exp v0.0.0-20240409090435-93d18d7e34b8 + golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 golang.org/x/oauth2 v0.19.0 google.golang.org/grpc v1.63.2 gopkg.in/yaml.v2 v2.4.0 @@ -152,15 +152,15 @@ require ( go.opentelemetry.io/otel/metric v1.25.0 // indirect go.opentelemetry.io/otel/trace v1.25.0 // indirect go.starlark.net v0.0.0-20240408152805-3f0a3703c02a // indirect - golang.org/x/crypto v0.22.0 // indirect + golang.org/x/crypto v0.23.0 // indirect golang.org/x/mod v0.17.0 // indirect - golang.org/x/net v0.24.0 // indirect + golang.org/x/net v0.25.0 // indirect golang.org/x/sync v0.7.0 // indirect - golang.org/x/sys v0.19.0 // indirect - golang.org/x/term v0.19.0 // indirect - golang.org/x/text v0.14.0 // indirect + golang.org/x/sys v0.20.0 // indirect + golang.org/x/term v0.20.0 // indirect + golang.org/x/text v0.15.0 // indirect golang.org/x/time v0.5.0 // indirect - golang.org/x/tools v0.20.0 // indirect + golang.org/x/tools v0.21.0 // indirect golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect google.golang.org/genproto v0.0.0-20240401170217-c3f982113cda // indirect google.golang.org/genproto/googleapis/api v0.0.0-20240401170217-c3f982113cda // indirect diff --git a/go.sum b/go.sum index 76f4baa5..fd39ba5e 100644 --- a/go.sum +++ b/go.sum @@ -649,10 +649,10 @@ github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kd github.com/apache/arrow/go/v10 v10.0.1/go.mod h1:YvhnlEePVnBS4+0z3fhPfUy7W1Ikj0Ih0vcRo/gZ1M0= github.com/apache/arrow/go/v11 v11.0.0/go.mod h1:Eg5OsL5H+e299f7u5ssuXsuHQVEGC4xei5aX110hRiI= github.com/apache/thrift v0.16.0/go.mod h1:PHK3hniurgQaNMZYaCLEqXKsYK8upmhPbmdP2FXSqgU= -github.com/argoproj/argo-cd/v2 v2.11.0-rc1 h1:VCjpw0bwPcULNJ/FG8BnIyeesyOpT+1MUWuXPskbsWQ= -github.com/argoproj/argo-cd/v2 v2.11.0-rc1/go.mod h1:/KySYrOzPQupCh7E1pzg6011p4AaRLszbUtkaWyATTU= -github.com/argoproj/gitops-engine v0.7.1-0.20240411122334-1ade3a199867 h1:zMATM3uzAQHBLJ142MEGrZ4+3+xsXT36hzB1Dj2jptE= -github.com/argoproj/gitops-engine v0.7.1-0.20240411122334-1ade3a199867/go.mod h1:gWE8uROi7hIkWGNAVM+8FWkMfo0vZ03SLx/aFw/DBzg= +github.com/argoproj/argo-cd/v2 v2.11.2 h1:NygNrTFIMWUe1b48ddUuH+q2vRTHB+dFk3NcErx6GcM= +github.com/argoproj/argo-cd/v2 v2.11.2/go.mod h1:nUOZqAT9f3GewdG/dzYgrpwqOSMj5ukoWw4yAV2/WXA= +github.com/argoproj/gitops-engine v0.7.1-0.20240416142647-fbecbb86e412 h1:je2wJpWtaoS55mA5MBPCeDnKMeF42pkxO9Oa5KbWrdg= +github.com/argoproj/gitops-engine v0.7.1-0.20240416142647-fbecbb86e412/go.mod h1:gWE8uROi7hIkWGNAVM+8FWkMfo0vZ03SLx/aFw/DBzg= github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1 h1:qsHwwOJ21K2Ao0xPju1sNuqphyMnMYkyB3ZLoLtxWpo= github.com/argoproj/pkg v0.13.7-0.20230626144333-d56162821bd1/go.mod h1:CZHlkyAD1/+FbEn6cB2DQTj48IoLGvEYsWEvtzP3238= github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio= @@ -1315,8 +1315,8 @@ golang.org/x/crypto v0.7.0/go.mod h1:pYwdfH91IfpZVANVyUOhSIPZaFoJGxTFbZhFTx+dXZU golang.org/x/crypto v0.9.0/go.mod h1:yrmDGqONDYtNj3tH8X9dzUun2m2lzPa9ngI6/RUPGR0= golang.org/x/crypto v0.10.0/go.mod h1:o4eNf7Ede1fv+hwOwZsTHl9EsPFO6q6ZvYR8vYfY45I= golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= -golang.org/x/crypto v0.22.0 h1:g1v0xeRhjcugydODzvb3mEM9SQ0HGp9s/nh3COQ/C30= -golang.org/x/crypto v0.22.0/go.mod h1:vr6Su+7cTlO45qkww3VDJlzDn0ctJvRgYbC2NvXHt+M= +golang.org/x/crypto v0.23.0 h1:dIJU/v2J8Mdglj/8rJ6UUOM3Zc9zLZxVZwwxMooUSAI= +golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -1332,8 +1332,8 @@ golang.org/x/exp v0.0.0-20200119233911-0405dc783f0a/go.mod h1:2RIsYlXP63K8oxa1u0 golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EHIKF9dgMWnmCNThgcyBT1FY9mM= golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20220827204233-334a2380cb91/go.mod h1:cyybsKvd6eL0RnXn6p/Grxp8F5bW7iYuBgsNCOHpMYE= -golang.org/x/exp v0.0.0-20240409090435-93d18d7e34b8 h1:ESSUROHIBHg7USnszlcdmjBEwdMj9VUvU+OPk4yl2mc= -golang.org/x/exp v0.0.0-20240409090435-93d18d7e34b8/go.mod h1:/lliqkxwWAhPjf5oSOIJup2XcqJaw8RGS6k3TGEc7GI= +golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842 h1:vr/HnozRka3pE4EsMEg1lgkXJkTFJCVUX+S/ZT6wYzM= +golang.org/x/exp v0.0.0-20240506185415-9bf2ced13842/go.mod h1:XtvwrStGgqGPLc4cjQfWqZHG1YFdYs6swckp8vpsjnc= golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= @@ -1445,8 +1445,8 @@ golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.8.0/go.mod h1:QVkue5JL9kW//ek3r6jTKnTFis1tRmNAW2P1shuFdJc= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= golang.org/x/net v0.11.0/go.mod h1:2L/ixqYpgIVXmeoSA/4Lu7BzTG4KIyPIryS4IsOd1oQ= -golang.org/x/net v0.24.0 h1:1PcaxkF854Fu3+lvBIx5SYn9wRlBzzcnHZSiaFFAb0w= -golang.org/x/net v0.24.0/go.mod h1:2Q7sJY5mzlzWjKtYUEXSlBWCdyaioyXzRB2RtU8KVE8= +golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac= +golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20190226205417-e64efc72b421/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45/go.mod h1:gOpvHmFTYa4IltrdGE7lF6nIHvwfUNPOp7c8zoXwtLw= @@ -1592,8 +1592,8 @@ golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.9.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.19.0 h1:q5f1RH2jigJ1MoAWp2KTp3gm5zAGFUTarQZ5U386+4o= -golang.org/x/sys v0.19.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= +golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y= +golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= @@ -1605,8 +1605,8 @@ golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo= golang.org/x/term v0.9.0/go.mod h1:M6DEAAIenWoTxdKrOltXcmDY3rSplQUkrvaDU5FcQyo= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= -golang.org/x/term v0.19.0 h1:+ThwsDv+tYfnJFhF4L8jITxu1tdTWRTZpdsWgEgjL6Q= -golang.org/x/term v0.19.0/go.mod h1:2CuTdWZ7KHSQwUzKva0cbMg6q2DMI3Mmxp+gKJbskEk= +golang.org/x/term v0.20.0 h1:VnkxpohqXaOBYJtBmEppKUG6mXpi+4O6purfc2+sMhw= +golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/text v0.0.0-20160726164857-2910a502d2bf/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -1625,8 +1625,9 @@ golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.8.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.10.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= -golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= +golang.org/x/text v0.15.0 h1:h1V/4gjBv8v9cjcR6+AR5+/cIYK5N/WAgiv4xlsEtAk= +golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ= @@ -1704,8 +1705,8 @@ golang.org/x/tools v0.2.0/go.mod h1:y4OqIKeOV/fWJetJ8bXPU1sEVniLMIyDAZWeHdV+NTA= golang.org/x/tools v0.3.0/go.mod h1:/rWhSS2+zyEVwoJf8YAX6L2f0ntZ7Kn/mGgAWcipA5k= golang.org/x/tools v0.4.0/go.mod h1:UE5sM2OK9E/d67R0ANs2xJizIymRP5gJU295PvKXxjQ= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.20.0 h1:hz/CVckiOxybQvFw6h7b/q80NTr9IUQb4s1IIzW7KNY= -golang.org/x/tools v0.20.0/go.mod h1:WvitBU7JJf6A4jOdg4S1tviW9bhUxkgeCui/0JHctQg= +golang.org/x/tools v0.21.0 h1:qc0xYgIbsSDt9EyWz05J5wfa7LOVW0YTLOXrqdLAWIw= +golang.org/x/tools v0.21.0/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/internal/pkg/argocd/argocd.go b/internal/pkg/argocd/argocd.go index b4552462..14ebc50b 100644 --- a/internal/pkg/argocd/argocd.go +++ b/internal/pkg/argocd/argocd.go @@ -7,7 +7,10 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "strconv" + "strings" + "time" cmdutil "github.com/argoproj/argo-cd/v2/cmd/util" "github.com/argoproj/argo-cd/v2/pkg/apiclient" @@ -16,6 +19,7 @@ import ( "github.com/argoproj/argo-cd/v2/pkg/apiclient/settings" 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" @@ -74,8 +78,9 @@ func generateArgocdAppDiff(ctx context.Context, app *argoappv1.Application, proj } ignoreAggregatedRoles := false + ignoreNormalizerOpts := normalizers.IgnoreNormalizerOpts{} diffConfig, err := argodiff.NewDiffConfigBuilder(). - WithDiffSettings(app.Spec.IgnoreDifferences, overrides, ignoreAggregatedRoles). + WithDiffSettings(app.Spec.IgnoreDifferences, overrides, ignoreAggregatedRoles, ignoreNormalizerOpts). WithTracking(argoSettings.AppLabelKey, argoSettings.TrackingMethod). WithNoCache(). Build() @@ -151,51 +156,111 @@ func createArgoCdClient() (apiclient.Client, error) { return clientset, nil } -func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranch string, repo string, appIf application.ApplicationServiceClient, projIf projectpkg.ProjectServiceClient, argoSettings *settings.Settings) (componentDiffResult DiffResult) { - componentDiffResult.ComponentPath = componentPath - +// 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. +// The SHA1 label is assumed to be populated by the ApplicationSet controller(or apps of apps or similar). +func findArgocdAppBySHA1Label(ctx context.Context, componentPath string, repo string, appClient application.ApplicationServiceClient) (app *argoappv1.Application, err error) { // Calculate sha1 of component path to use in a label selector cPathBa := []byte(componentPath) hasher := sha1.New() //nolint:gosec // G505: Blocklisted import crypto/sha1: weak cryptographic primitive (gosec), this is not a cryptographic use case hasher.Write(cPathBa) componentPathSha1 := hex.EncodeToString(hasher.Sum(nil)) - - // Find ArgoCD application by the path SHA1 label selector and repo name - // That label is assumed to be pupulated by the ApplicationSet controller(or apps of apps or similar). labelSelector := fmt.Sprintf("telefonistka.io/component-path-sha1=%s", componentPathSha1) log.Debugf("Using label selector: %s", labelSelector) appLabelQuery := application.ApplicationQuery{ Selector: &labelSelector, Repo: &repo, } - foundApps, err := appIf.List(ctx, &appLabelQuery) + foundApps, err := appClient.List(ctx, &appLabelQuery) if err != nil { - log.Errorf("Error listing ArgoCD applications: %v", err) - componentDiffResult.DiffError = err - return componentDiffResult + return nil, fmt.Errorf("Error listing ArgoCD applications: %v", err) } if len(foundApps.Items) == 0 { - componentDiffResult.DiffError = fmt.Errorf("No ArgoCD application found for component path %s(repo %s), used this label selector: %s", componentPath, repo, labelSelector) + return nil, fmt.Errorf("No ArgoCD application found for component path sha1 %s(repo %s), used this label selector: %s", componentPathSha1, repo, labelSelector) + } + + // we expect only one app with this label and repo selectors + return &foundApps.Items[0], nil +} + +// findArgocdAppByManifestPathAnnotation is the default method to find an ArgoCD application by the manifest-generate-paths annotation. +// It assumes the ArgoCD (optional) manifest-generate-paths annotation is set on all relevant apps. +// Notice that this method includes a full list of all ArgoCD applications in the repo, this could be a performance issue if there are many apps in the repo. +func findArgocdAppByManifestPathAnnotation(ctx context.Context, componentPath string, repo string, appClient application.ApplicationServiceClient) (app *argoappv1.Application, err error) { + // argocd.argoproj.io/manifest-generate-paths + appQuery := application.ApplicationQuery{ + Repo: &repo, + } + // AFAIKT I can't use standard grpc instrumentation here, since the argocd client abstracts too much (including the choice between Grpc and Grpc-web) + // I'll just manually log the time it takes to get the apps for now + getAppsStart := time.Now() + allRepoApps, err := appClient.List(ctx, &appQuery) + getAppsDuration := time.Since(getAppsStart).Milliseconds() + log.Infof("Got %v ArgoCD applications for repo %s in %v ms", len(allRepoApps.Items), repo, getAppsDuration) + if err != nil { + return nil, err + } + for _, app := range allRepoApps.Items { + // Check if the app has the annotation + // https://argo-cd.readthedocs.io/en/stable/operator-manual/high_availability/#manifest-paths-annotation + // Consider the annotation content can a semi-colon separated list of paths, an absolute path or a relative path(start with a ".") and the manifest-paths-annotation could be a subpath of componentPath. + // We need to check if the annotation is a subpath of componentPath + + appManifestPathsAnnotation := app.Annotations["argocd.argoproj.io/manifest-generate-paths"] + + for _, manifetsPathElement := range strings.Split(appManifestPathsAnnotation, ";") { + // if `manifest-generate-paths` element starts with a "." it is a relative path(relative to repo root), we need to join it with the app source path + if strings.HasPrefix(manifetsPathElement, ".") { + manifetsPathElement = filepath.Join(app.Spec.Source.Path, manifetsPathElement) + } + + // Checking is componentPath is a subpath of the manifetsPathElement + // Using filepath.Rel solves all kinds of path issues, like double slashes, etc. + rel, err := filepath.Rel(manifetsPathElement, componentPath) + if !strings.HasPrefix(rel, "..") && err == nil { + log.Debugf("Found app %s with manifest-generate-paths(\"%s\") annotation that matches %s", app.Name, appManifestPathsAnnotation, componentPath) + return &app, nil + } + } + } + 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 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 + + // Find ArgoCD application by the path SHA1 label selector and repo name + // At the moment we assume one to one mapping between Telefonistka components and ArgoCD application + + var foundApp *argoappv1.Application + var err error + if useSHALabelForArgoDicovery { + foundApp, err = findArgocdAppBySHA1Label(ctx, componentPath, repo, appClient) + } else { + foundApp, err = findArgocdAppByManifestPathAnnotation(ctx, componentPath, repo, appClient) + } + if err != nil { + componentDiffResult.DiffError = err return componentDiffResult } - log.Debugf("Found ArgoCD application: %s", foundApps.Items[0].Name) + log.Debugf("Found ArgoCD application: %s", foundApp.Name) // Get the application and its resources, resources are the live state of the application objects. + // The 2nd "app fetch" is needed for the "refreshTypeHArd", we don't want to do that to non-relevant apps" refreshType := string(argoappv1.RefreshTypeHard) appNameQuery := application.ApplicationQuery{ - Name: &foundApps.Items[0].Name, // we expect only one app with this label and repo selectors + Name: &foundApp.Name, // we expect only one app with this label and repo selectors Refresh: &refreshType, } - app, err := appIf.Get(ctx, &appNameQuery) + app, err := appClient.Get(ctx, &appNameQuery) if err != nil { componentDiffResult.DiffError = err - log.Errorf("Error getting app %s: %v", foundApps.Items[0].Name, err) + log.Errorf("Error getting app %s: %v", foundApp.Name, err) return componentDiffResult } log.Debugf("Got ArgoCD app %s", app.Name) componentDiffResult.ArgoCdAppName = app.Name componentDiffResult.ArgoCdAppURL = fmt.Sprintf("%s/applications/%s", argoSettings.URL, app.Name) - resources, err := appIf.ManagedResources(ctx, &application.ResourcesQuery{ApplicationName: &app.Name, AppNamespace: &app.Namespace}) + resources, err := appClient.ManagedResources(ctx, &application.ResourcesQuery{ApplicationName: &app.Name, AppNamespace: &app.Namespace}) if err != nil { componentDiffResult.DiffError = err log.Errorf("Error getting (live)resources for app %s: %v", app.Name, err) @@ -211,7 +276,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc Revision: &prBranch, AppNamespace: &app.Namespace, } - manifests, err := appIf.GetManifests(ctx, &manifestQuery) + manifests, err := appClient.GetManifests(ctx, &manifestQuery) if err != nil { componentDiffResult.DiffError = err log.Errorf("Error getting manifests for app %s, revision %s: %v", app.Name, prBranch, err) @@ -222,7 +287,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc diffOption.revision = prBranch // Now we diff the live state(resources) and target state of the application objects(diffOption.res) - detailedProject, err := projIf.GetDetailedProject(ctx, &projectpkg.ProjectQuery{Name: app.Spec.Project}) + detailedProject, err := projClient.GetDetailedProject(ctx, &projectpkg.ProjectQuery{Name: app.Spec.Project}) if err != nil { componentDiffResult.DiffError = err log.Errorf("Error getting project %s: %v", app.Spec.Project, err) @@ -238,7 +303,7 @@ func generateDiffOfAComponent(ctx context.Context, componentPath string, prBranc } // GenerateDiffOfChangedComponents generates diff of changed components -func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []string, prBranch string, repo string) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) { +func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []string, prBranch string, repo string, useSHALabelForArgoDicovery bool) (hasComponentDiff bool, hasComponentDiffErrors bool, diffResults []DiffResult, err error) { hasComponentDiff = false hasComponentDiffErrors = false // env var should be centralized @@ -248,27 +313,27 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st return false, true, nil, err } - conn, appIf, err := client.NewApplicationClient() + 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, projIf, err := client.NewProjectClient() + 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, settingsIf, err := client.NewSettingsClient() + 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 := settingsIf.Get(ctx, &settings.SettingsQuery{}) + argoSettings, err := settingClient.Get(ctx, &settings.SettingsQuery{}) if err != nil { log.Errorf("Error getting ArgoCD settings: %v", err) return false, true, nil, err @@ -276,7 +341,7 @@ func GenerateDiffOfChangedComponents(ctx context.Context, componentPathList []st log.Debugf("Checking ArgoCD diff for components: %v", componentPathList) for _, componentPath := range componentPathList { - currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appIf, projIf, argoSettings) + currentDiffResult := generateDiffOfAComponent(ctx, componentPath, prBranch, repo, appClient, projClient, 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/argocd/argocd_test.go b/internal/pkg/argocd/argocd_test.go new file mode 100644 index 00000000..3bda870c --- /dev/null +++ b/internal/pkg/argocd/argocd_test.go @@ -0,0 +1,187 @@ +package argocd + +import ( + "context" + "testing" + + argoappv1 "github.com/argoproj/argo-cd/v2/pkg/apis/application/v1alpha1" + "github.com/golang/mock/gomock" + "github.com/wayfair-incubator/telefonistka/internal/pkg/mocks" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" +) + +func TestFindArgocdAppBySHA1Label(t *testing.T) { + // Here the filtering is done on the ArgoCD server side, so we are just testing the function returns a app + t.Parallel() + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockApplicationClient := mocks.NewMockApplicationServiceClient(ctrl) + expectedResponse := &argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{ + "telefonistka.io/component-path-sha1": "111111", + }, + Name: "right-app", + }, + }, + }, + } + + mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) + + app, err := findArgocdAppBySHA1Label(ctx, "random/path", "some-repo", mockApplicationClient) + if err != nil { + t.Errorf("Error: %v", err) + } + if app.Name != "right-app" { + t.Errorf("App name is not right-app") + } +} + +func TestFindArgocdAppByPathAnnotation(t *testing.T) { + t.Parallel() + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockApplicationClient := mocks.NewMockApplicationServiceClient(ctrl) + expectedResponse := &argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": "wrong/path/", + }, + Name: "wrong-app", + }, + }, + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": "right/path/", + }, + Name: "right-app", + }, + }, + }, + } + + mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) + + apps, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient) + if err != nil { + t.Errorf("Error: %v", err) + } + t.Logf("apps: %v", apps) +} + +// Here I'm testing a ";" delimted path annotation +func TestFindArgocdAppByPathAnnotationSemiColon(t *testing.T) { + t.Parallel() + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockApplicationClient := mocks.NewMockApplicationServiceClient(ctrl) + expectedResponse := &argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": "wrong/path/;wrong/path2/", + }, + Name: "wrong-app", + }, + }, + { // This is the app we want to find - it has the right path as one of the elements in the annotation + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": "wrong/path/;right/path/", + }, + Name: "right-app", + }, + }, + }, + } + + mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) + + app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient) + if err != nil { + t.Errorf("Error: %v", err) + } + if app.Name != "right-app" { + t.Errorf("App name is not right-app") + } +} + +// Here I'm testing a "." path annotation - this is a special case where the path is relative to the repo root specified in the application .spec +func TestFindArgocdAppByPathAnnotationRelative(t *testing.T) { + t.Parallel() + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockApplicationClient := mocks.NewMockApplicationServiceClient(ctrl) + expectedResponse := &argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": ".", + }, + Name: "right-app", + }, + Spec: argoappv1.ApplicationSpec{ + Source: &argoappv1.ApplicationSource{ + RepoURL: "", + Path: "right/path", + }, + }, + }, + }, + } + + mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) + app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient) + if err != nil { + t.Errorf("Error: %v", err) + } else if app.Name != "right-app" { + t.Errorf("App name is not right-app") + } +} + +// Here I'm testing a "." path annotation - this is a special case where the path is relative to the repo root specified in the application .spec +func TestFindArgocdAppByPathAnnotationRelative2(t *testing.T) { + t.Parallel() + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockApplicationClient := mocks.NewMockApplicationServiceClient(ctrl) + expectedResponse := &argoappv1.ApplicationList{ + Items: []argoappv1.Application{ + { + ObjectMeta: metav1.ObjectMeta{ + Annotations: map[string]string{ + "argocd.argoproj.io/manifest-generate-paths": "./path", + }, + Name: "right-app", + }, + Spec: argoappv1.ApplicationSpec{ + Source: &argoappv1.ApplicationSource{ + RepoURL: "", + Path: "right/", + }, + }, + }, + }, + } + + mockApplicationClient.EXPECT().List(gomock.Any(), gomock.Any()).Return(expectedResponse, nil) + app, err := findArgocdAppByManifestPathAnnotation(ctx, "right/path", "some-repo", mockApplicationClient) + if err != nil { + t.Errorf("Error: %v", err) + } else if app.Name != "right-app" { + t.Errorf("App name is not right-app") + } +} diff --git a/internal/pkg/configuration/config.go b/internal/pkg/configuration/config.go index ea822829..379d9c8c 100644 --- a/internal/pkg/configuration/config.go +++ b/internal/pkg/configuration/config.go @@ -35,13 +35,14 @@ 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"` - CommentArgocdDiffonPR bool `yaml:"commentArgocdDiffonPR"` - AutoMergeNoDiffPRs bool `yaml:"autoMergeNoDiffPRs"` + PromtionPrLables []string `yaml:"promtionPRlables"` + DryRunMode bool `yaml:"dryRunMode"` + AutoApprovePromotionPrs bool `yaml:"autoApprovePromotionPrs"` + ToggleCommitStatus map[string]string `yaml:"toggleCommitStatus"` + WebhookEndpointRegexs []WebhookEndpointRegex `yaml:"webhookEndpointRegexs"` + CommentArgocdDiffonPR bool `yaml:"commentArgocdDiffonPR"` + AutoMergeNoDiffPRs bool `yaml:"autoMergeNoDiffPRs"` + UseSHALabelForArgoDicovery bool `yaml:"useSHALabelForArgoDicovery"` } func ParseConfigFromYaml(y string) (*Config, error) { diff --git a/internal/pkg/githubapi/github.go b/internal/pkg/githubapi/github.go index 951e7ca0..a7e3cce7 100644 --- a/internal/pkg/githubapi/github.go +++ b/internal/pkg/githubapi/github.go @@ -22,6 +22,7 @@ import ( "github.com/wayfair-incubator/telefonistka/internal/pkg/argocd" cfg "github.com/wayfair-incubator/telefonistka/internal/pkg/configuration" prom "github.com/wayfair-incubator/telefonistka/internal/pkg/prometheus" + "golang.org/x/exp/maps" ) type promotionInstanceMetaData struct { @@ -49,6 +50,7 @@ type GhPrClientDetails struct { type prMetadata struct { OriginalPrAuthor string `json:"originalPrAuthor"` OriginalPrNumber int `json:"originalPrNumber"` + PromotedPaths []string `json:"promotedPaths"` PreviousPromotionMetadata map[int]promotionInstanceMetaData `json:"previousPromotionPaths"` } @@ -107,12 +109,21 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr ghPrClientDetails.PrLogger.Errorf("Failed to minimize stale comments: err=%s\n", err) } if config.CommentArgocdDiffonPR { - componentPathList, err := generateListOfChangedComponentPaths(ghPrClientDetails, config) - if err != nil { - prHandleError = err - ghPrClientDetails.PrLogger.Errorf("Failed to get list of changed components: err=%s\n", err) + var componentPathList []string + + // Promotion PR have the list of paths to promote in the PR metadata + // For non promotion PR, we will generate the list of changed components based the PR changed files and the telefonistka configuration(sourcePath) + if DoesPrHasLabel(*eventPayload, "promotion") { + componentPathList = ghPrClientDetails.PrMetadata.PromotedPaths + } else { + componentPathList, err = generateListOfChangedComponentPaths(ghPrClientDetails, config) + if err != nil { + prHandleError = err + 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) + + hasComponentDiff, hasComponentDiffErrors, diffOfChangedComponents, err := argocd.GenerateDiffOfChangedComponents(ctx, componentPathList, ghPrClientDetails.Ref, ghPrClientDetails.RepoURL, config.UseSHALabelForArgoDicovery) if err != nil { prHandleError = err ghPrClientDetails.PrLogger.Errorf("Failed to get ArgoCD diff information: err=%s\n", err) @@ -127,7 +138,9 @@ func HandlePREvent(eventPayload *github.PullRequestEvent, ghPrClientDetails GhPr } else { ghPrClientDetails.PrLogger.Debugf("PR %v labeled\n%+v", *eventPayload.PullRequest.Number, prLables) } - if DoesPrHasLabel(*eventPayload, "promotion") && config.AutoMergeNoDiffPRs { + // If the PR is a promotion PR and the diff is empty, we can auto-merge it + // "len(componentPathList) > 0" validates we are not auto-merging a PR that we failed to understand which apps it affects + if DoesPrHasLabel(*eventPayload, "promotion") && config.AutoMergeNoDiffPRs && len(componentPathList) > 0 { ghPrClientDetails.PrLogger.Infof("Auto-merging (no diff) PR %d", *eventPayload.PullRequest.Number) err := MergePr(ghPrClientDetails, eventPayload.PullRequest.Number) if err != nil { @@ -860,6 +873,8 @@ func generatePromotionPrBody(ghPrClientDetails GhPrClientDetails, components str // newPrMetadata.PreviousPromotionMetadata[ghPrClientDetails.PrNumber].TargetPaths = targetPaths // newPrMetadata.PreviousPromotionMetadata[ghPrClientDetails.PrNumber].SourcePath = sourcePath + newPrMetadata.PromotedPaths = maps.Keys(promotion.ComputedSyncPaths) + newPrBody = fmt.Sprintf("Promotion path(%s):\n\n", components) keys := make([]int, 0) diff --git a/internal/pkg/mocks/.gitignore b/internal/pkg/mocks/.gitignore new file mode 100644 index 00000000..191fc316 --- /dev/null +++ b/internal/pkg/mocks/.gitignore @@ -0,0 +1 @@ +/argocd_application.go diff --git a/internal/pkg/mocks/mocks.go b/internal/pkg/mocks/mocks.go new file mode 100644 index 00000000..0fbed2e1 --- /dev/null +++ b/internal/pkg/mocks/mocks.go @@ -0,0 +1,5 @@ +package mocks + +// This package contains generated mocks + +//go:generate go run github.com/golang/mock/mockgen@v1.6.0 -destination=argocd_application.go -package=mocks github.com/argoproj/argo-cd/v2/pkg/apiclient/application ApplicationServiceClient