Merge pull request #212 from commercetools/main

Sync from fork

.github/workflows/lint.yml

@@ -39,7 +39,7 @@ jobs:
         uses: golangci/golangci-lint-action@v6
         with:
           # Optional: version of golangci-lint to use in form of v1.2 or v1.2.3 or `latest` to use the latest version
-          version: v1.57.2
+          version: v1.60.3
 
           # Optional: working directory, useful for monorepos
           # working-directory: somedir

.gitignore

@@ -1,2 +1,4 @@
 /telefonistka
 vendor/
+internal/pkg/mocks/argocd_settings.go
+internal/pkg/mocks/argocd_project.go

.markdownlint.json

@@ -12,5 +12,6 @@
     "MD025": {
         "front_matter_title": ""
     },
-    "MD041": false
+    "MD041": false,
+	"MD034": false
 }

Makefile

@@ -25,5 +25,5 @@ clean:
 
 .PHONY: test
 test: $(VENDOR_DIR)
-	go test -v -timeout 30s ./...
+	TEMPLATES_PATH=../../../templates/ go test -v -timeout 30s ./...
 

README.md

@@ -169,11 +169,34 @@ See [here](docs/observability.md)
 
 ## Development
 
+### Local Testing
+
+Telefonistka have 3 major methods to interact with the world:
+
+* Receive event webhooks from GitHub
+* Send API calls to GitHub REST and GraphQL APIs(requires network access and credentials)
+* Send API calls to ArgoCD API(requires network access and credentials)
+
+Supporting all those requirements in a local environment might require lots of setup.
+Assuming you have a working lab environment, the easiest way to locally test Telefonistka might be with tools like [mirrord](https://mirrord.dev/) or [telepresence](https://www.telepresence.io/)
+
+A [mirrord.json](mirrord.json) is supplied as reference.
+
+This is how I compile and trigger mirrord execution
+
+```sh
+go build . && mirrord exec -f mirrord.json ./telefonistka server
+```
+
+Alternatively, you can use `ngrok` or similar services to route webhook to a local instance, but you still need to provide credentials to all outbound API calls.
+
 * use Ngrok ( `ngrok http 8080` ) to expose the local instance
 * See the URLs in ngrok command output.
 * Add a webhook to repo setting (don't forget the `/webhook` path in the URL).
 * Content type needs to be `application/json`, **currently** only PR events are needed
 
+### Building Container Image From Forks
+
 To publish container images from a forked repo set the `IMAGE_NAME` and `REGISTRY` GitHub Action Repository variables to use GitHub packages.
 `REGISTRY` should be `ghcr.io` and `IMAGE_NAME` should match the repository slug, like so:
 like so:

cmd/telefonistka/bump-version-overwrite.go

@@ -73,7 +73,7 @@ func bumpVersionOverwrite(targetRepo string, targetFile string, file string, git
 	ghPrClientDetails.PrLogger = log.WithFields(log.Fields{}) // TODO what fields should be here?
 
 	defaultBranch, _ := ghPrClientDetails.GetDefaultBranch()
-	initialFileContent, err, statusCode := githubapi.GetFileContent(ghPrClientDetails, defaultBranch, targetFile)
+	initialFileContent, statusCode, err := githubapi.GetFileContent(ghPrClientDetails, defaultBranch, targetFile)
 	if statusCode == 404 {
 		ghPrClientDetails.PrLogger.Infof("File %s was not found\n", targetFile)
 	} else if err != nil {

cmd/telefonistka/bump-version-regex.go

@@ -71,7 +71,7 @@ func bumpVersionRegex(targetRepo string, targetFile string, regex string, replac
 	r := regexp.MustCompile(regex)
 	defaultBranch, _ := ghPrClientDetails.GetDefaultBranch()
 
-	initialFileContent, err, _ := githubapi.GetFileContent(ghPrClientDetails, defaultBranch, targetFile)
+	initialFileContent, _, err := githubapi.GetFileContent(ghPrClientDetails, defaultBranch, targetFile)
 	if err != nil {
 		ghPrClientDetails.PrLogger.Errorf("Fail to fetch file content:%s\n", err)
 		os.Exit(1)

cmd/telefonistka/bump-version-yaml.go

@@ -74,7 +74,7 @@ func bumpVersionYaml(targetRepo string, targetFile string, address string, value
 
 	defaultBranch, _ := ghPrClientDetails.GetDefaultBranch()
 
-	initialFileContent, err, _ := githubapi.GetFileContent(ghPrClientDetails, defaultBranch, targetFile)
+	initialFileContent, _, err := githubapi.GetFileContent(ghPrClientDetails, defaultBranch, targetFile)
 	if err != nil {
 		ghPrClientDetails.PrLogger.Errorf("Fail to fetch file content:%s\n", err)
 		os.Exit(1)

cmd/telefonistka/event.go

@@ -1,14 +1,9 @@
 package telefonistka
 
 import (
-	"bytes"
-	"context"
-	"io"
-	"net/http"
 	"os"
 
 	lru "github.com/hashicorp/golang-lru/v2"
-	log "github.com/sirupsen/logrus"
 	"github.com/spf13/cobra"
 	"github.com/wayfair-incubator/telefonistka/internal/pkg/githubapi"
 )
@@ -32,27 +27,9 @@ func init() { //nolint:gochecknoinits
 }
 
 func event(eventType string, eventFilePath string) {
-	ctx := context.Background()
-
-	log.Infof("Event type: %s", eventType)
-	log.Infof("Proccesing file: %s", eventFilePath)
-
-	payload, err := os.ReadFile(eventFilePath)
-	if err != nil {
-		panic(err)
-	}
-
-	// To use the same code path as for Webhook I'm creating an http request with the payload from the file.
-	// This might not be very smart.
-
-	h, _ := http.NewRequest("POST", "", nil) //nolint:noctx
-	h.Body = io.NopCloser(bytes.NewReader(payload))
-	h.Header.Set("Content-Type", "application/json")
-	h.Header.Set("X-GitHub-Event", eventType)
-
 	mainGhClientCache, _ := lru.New[string, githubapi.GhClientPair](128)
 	prApproverGhClientCache, _ := lru.New[string, githubapi.GhClientPair](128)
-	githubapi.HandleEvent(h, ctx, mainGhClientCache, prApproverGhClientCache, nil)
+	githubapi.ReciveEventFile(eventFilePath, eventType, mainGhClientCache, prApproverGhClientCache)
 }
 
 func getEnv(key, fallback string) string {

cmd/telefonistka/server.go

@@ -1,7 +1,6 @@
 package telefonistka
 
 import (
-	"context"
 	"net/http"
 	"os"
 	"time"
@@ -39,9 +38,13 @@ func init() { //nolint:gochecknoinits
 
 func handleWebhook(githubWebhookSecret []byte, mainGhClientCache *lru.Cache[string, githubapi.GhClientPair], prApproverGhClientCache *lru.Cache[string, githubapi.GhClientPair]) func(http.ResponseWriter, *http.Request) {
 	return func(w http.ResponseWriter, r *http.Request) {
-		ctx, cancel := context.WithTimeout(r.Context(), 30*time.Second)
-		defer cancel()
-		githubapi.HandleEvent(r, ctx, mainGhClientCache, prApproverGhClientCache, githubWebhookSecret)
+		err := githubapi.ReciveWebhook(r, mainGhClientCache, prApproverGhClientCache, githubWebhookSecret)
+		if err != nil {
+			log.Errorf("error handling webhook: %v", err)
+			http.Error(w, "Internal server error", http.StatusInternalServerError)
+			return
+		}
+		w.WriteHeader(http.StatusOK)
 	}
 }
 
@@ -54,6 +57,8 @@ func serve() {
 	mainGhClientCache, _ := lru.New[string, githubapi.GhClientPair](128)
 	prApproverGhClientCache, _ := lru.New[string, githubapi.GhClientPair](128)
 
+	go githubapi.MainGhMetricsLoop(mainGhClientCache)
+
 	mux := http.NewServeMux()
 	mux.HandleFunc("/webhook", handleWebhook(githubWebhookSecret, mainGhClientCache, prApproverGhClientCache))
 	mux.Handle("/metrics", promhttp.Handler())

docs/installation.md

@@ -91,6 +91,12 @@ Environment variables for the webhook process:
 
 `TEMPLATES_PATH` Telefonistka uses Go templates to format GitHub PR comments, the variable override the default templates path("templates/"), useful for environments where the container workdir is overridden(like GitHub Actions) or when custom templates are desired.
 
+`CUSTOM_COMMIT_STATUS_URL_TEMPLATE_PATH` allows you to set a custom [commit status](https://docs.github.com/en/rest/commits/statuses?apiVersion=2022-11-28#about-commit-statuses) target URL using Go templates. The commit time will be passed as a dynamic parameter to the template. Here is an example:
+
+```console
+https://custom-url.com?time={{.CommitTime}}
+```
+
 `ARGOCD_SERVER_ADDR` Hostname and port of the ArgoCD API endpoint, like `argocd-server.argocd.svc.cluster.local:443`, default is `localhost:8080"`
 
 `ARGOCD_TOKEN` ArgoCD authentication token.
@@ -123,9 +129,11 @@ Configuration keys:
 |`autoApprovePromotionPrs`| if true the bot will auto-approve all promotion PRs, with the assumption the original PR was peer reviewed and is promoted verbatim. Required additional GH token via APPROVER_GITHUB_OAUTH_TOKEN env variable|
 |`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|
 |`whProxtSkipTLSVerifyUpstream`| This disables upstream TLS server certificate validation for the webhook proxy functionality. Default is `false`. |
-|`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`.|
+|`argocd.commentDiffonPR`| 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. |
+|`argocd.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)|
+|`argocd.useSHALabelForAppDiscovery`| 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`.|
+|`argocd.allowSyncfromBranchPathRegex`| 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.|
+|`argocd.createTempAppObjectFromNewApps`| For application created in PR Telefonistka needs to create a temporary ArgoCD Application Object to render the manifests, this key enables this behavior. The application spec is pulled from a Matching ApplicationSet object and the temporary object is deleted after the manifests are rendered. This feature currently support ApplicationSets with Git **Directory** generator|
 <!-- markdownlint-enable MD033 -->
 
 Example:
@@ -171,8 +179,12 @@ promotionPaths:
         - "clusters/prod/us-east4/c2"
 dryRunMode: true
 autoApprovePromotionPrs: true
-commentArgocdDiffonPR: true
-autoMergeNoDiffPRs: true
+argocd:
+  commentDiffonPR: true
+  autoMergeNoDiffPRs: true
+  allowSyncfromBranchPathRegex: '^workspace/.*$'
+  useSHALabelForAppDiscovery: true
+  createTempAppObjectFromNewApps: true
 toggleCommitStatus:
   override-terrafrom-pipeline: "github-action-terraform"
 ```
@@ -182,20 +194,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

docs/observability.md

@@ -6,6 +6,13 @@
 |telefonistka_github_github_rest_api_client_rate_remaining|gauge|The number of remaining requests the client can make this hour||
 |telefonistka_github_github_rest_api_client_rate_limit|gauge|The number of requests per hour the client is currently limited to||
 |telefonistka_webhook_server_webhook_hits_total|counter|The total number of validated webhook hits|`parsing`|
+|telefonistka_github_open_prs|gauge|The number of open PRs|`repo_slug`|
+|telefonistka_github_open_promotion_prs|gauge|The number of open promotion PRs|`repo_slug`|
+|telefonistka_github_open_prs_with_pending_telefonistka_checks|gauge|The number of open PRs with pending Telefonistka checks(excluding PRs with very recent commits)|`repo_slug`|
+|telefonistka_github_commit_status_updates_total|counter|The total number of commit status updates, and their status (success/pending/failure)|`repo_slug`, `status`|
+
+> [!NOTE]  
+> telefonistka_github_*_prs metrics are only supported on installtions that uses GitHub App authentication as it provides an easy way to query the relevant GH repos.
 
 Example metrics snippet:
 
@@ -26,4 +33,20 @@ telefonistka_github_github_rest_api_client_rate_remaining 99668
 # HELP telefonistka_webhook_server_webhook_hits_total The total number of validated webhook hits
 # TYPE telefonistka_webhook_server_webhook_hits_total counter
 telefonistka_webhook_server_webhook_hits_total{parsing="successful"} 8
+# HELP telefonistka_github_commit_status_updates_total The total number of commit status updates, and their status (success/pending/failure)
+# TYPE telefonistka_github_commit_status_updates_total counter
+telefonistka_github_commit_status_updates_total{repo_slug="foo/bar2",status="error"} 1
+telefonistka_github_commit_status_updates_total{repo_slug="foo/bar2",status="pending"} 1
+# HELP telefonistka_github_open_promotion_prs The total number of open PRs with promotion label
+# TYPE telefonistka_github_open_promotion_prs gauge
+telefonistka_github_open_promotion_prs{repo_slug="foo/bar1"} 0
+telefonistka_github_open_promotion_prs{repo_slug="foo/bar2"} 10
+# HELP telefonistka_github_open_prs The total number of open PRs
+# TYPE telefonistka_github_open_prs gauge
+telefonistka_github_open_prs{repo_slug="foo/bar1"} 0
+telefonistka_github_open_prs{repo_slug="foo/bar2"} 21
+# HELP telefonistka_github_open_prs_with_pending_telefonistka_checks The total number of open PRs with pending Telefonistka checks(excluding PRs with very recent commits)
+# TYPE telefonistka_github_open_prs_with_pending_telefonistka_checks gauge
+telefonistka_github_open_prs_with_pending_telefonistka_checks{repo_slug="foo/bar1"} 0
+telefonistka_github_open_prs_with_pending_telefonistka_checks{repo_slug="foo/bar2"} 0
 ```