From 7da6b961dacdf5838e1cac587280ab1970e1a4fb Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 21 Feb 2025 22:11:33 +0100 Subject: [PATCH 1/2] feat: allow opt-in to credentials over HTTP Signed-off-by: Hidde Beydals --- internal/credentials/kubernetes/database.go | 3 +- .../credentials/kubernetes/database_test.go | 47 ++++++++++++++----- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/internal/credentials/kubernetes/database.go b/internal/credentials/kubernetes/database.go index 8585a76d0..436ef8447 100644 --- a/internal/credentials/kubernetes/database.go +++ b/internal/credentials/kubernetes/database.go @@ -35,6 +35,7 @@ type database struct { // of the credentials.Database interface. type DatabaseConfig struct { GlobalCredentialsNamespaces []string `envconfig:"GLOBAL_CREDENTIALS_NAMESPACES" default:""` + AllowCredentialsOverHTTP bool `envconfig:"ALLOW_CREDENTIALS_OVER_HTTP" default:"false"` } func DatabaseConfigFromEnv() DatabaseConfig { @@ -81,7 +82,7 @@ func (k *database) Get( ) (credentials.Credentials, bool, error) { // If we are dealing with an insecure HTTP endpoint (of any type), // refuse to return any credentials - if strings.HasPrefix(repoURL, "http://") { + if !k.cfg.AllowCredentialsOverHTTP && strings.HasPrefix(repoURL, "http://") { logging.LoggerFromContext(ctx).Info( "refused to get credentials for insecure HTTP endpoint", "repoURL", repoURL, diff --git a/internal/credentials/kubernetes/database_test.go b/internal/credentials/kubernetes/database_test.go index 59a87fc0b..b65792113 100644 --- a/internal/credentials/kubernetes/database_test.go +++ b/internal/credentials/kubernetes/database_test.go @@ -180,6 +180,7 @@ func TestGet(t *testing.T) { testCases := []struct { name string secrets []client.Object + cfg DatabaseConfig credType credentials.Type repoURL string expected *corev1.Secret @@ -199,15 +200,21 @@ func TestGet(t *testing.T) { expected: projectGitCredentialWithRepoURLPattern, }, { - name: "git URL exact match in global namespace", - secrets: []client.Object{globalGitCredentialWithRepoURL}, + name: "git URL exact match in global namespace", + secrets: []client.Object{globalGitCredentialWithRepoURL}, + cfg: DatabaseConfig{ + GlobalCredentialsNamespaces: []string{testGlobalNamespace}, + }, credType: credentials.TypeGit, repoURL: testGitRepoURL, expected: globalGitCredentialWithRepoURL, }, { - name: "git URL pattern match in global namespace", - secrets: []client.Object{globalGitCredentialWithRepoURLPattern}, + name: "git URL pattern match in global namespace", + secrets: []client.Object{globalGitCredentialWithRepoURLPattern}, + cfg: DatabaseConfig{ + GlobalCredentialsNamespaces: []string{testGlobalNamespace}, + }, credType: credentials.TypeGit, repoURL: testGitRepoURL, expected: globalGitCredentialWithRepoURLPattern, @@ -231,15 +238,21 @@ func TestGet(t *testing.T) { expected: projectImageCredentialWithRepoURLPattern, }, { - name: "image URL exact match in global namespace", - secrets: []client.Object{globalImageCredentialWithRepoURL}, + name: "image URL exact match in global namespace", + secrets: []client.Object{globalImageCredentialWithRepoURL}, + cfg: DatabaseConfig{ + GlobalCredentialsNamespaces: []string{testGlobalNamespace}, + }, credType: credentials.TypeImage, repoURL: testImageURL, expected: globalImageCredentialWithRepoURL, }, { - name: "image URL pattern match in global namespace", - secrets: []client.Object{globalImageCredentialWithRepoURLPattern}, + name: "image URL pattern match in global namespace", + secrets: []client.Object{globalImageCredentialWithRepoURLPattern}, + cfg: DatabaseConfig{ + GlobalCredentialsNamespaces: []string{testGlobalNamespace}, + }, credType: credentials.TypeImage, repoURL: testImageURL, expected: globalImageCredentialWithRepoURLPattern, @@ -262,6 +275,9 @@ func TestGet(t *testing.T) { globalGitCredentialWithRepoURL, globalGitCredentialWithRepoURLPattern, }, + cfg: DatabaseConfig{ + GlobalCredentialsNamespaces: []string{testGlobalNamespace}, + }, credType: credentials.TypeGit, repoURL: testGitRepoURL, expected: globalGitCredentialWithRepoURL, @@ -296,6 +312,17 @@ func TestGet(t *testing.T) { repoURL: testInsecureGitURL, expected: nil, }, + { + name: "insecure HTTP endpoint allowed", + // Matches because credentials for insecure URLs are allowed + secrets: []client.Object{projectGitCredentialWithInsecureRepoURL}, + cfg: DatabaseConfig{ + AllowCredentialsOverHTTP: true, + }, + credType: credentials.TypeGit, + repoURL: testInsecureGitURL, + expected: projectGitCredentialWithInsecureRepoURL, + }, } for _, testCase := range testCases { @@ -303,9 +330,7 @@ func TestGet(t *testing.T) { creds, found, err := NewDatabase( context.Background(), fake.NewClientBuilder().WithObjects(testCase.secrets...).Build(), - DatabaseConfig{ - GlobalCredentialsNamespaces: []string{testGlobalNamespace}, - }, + testCase.cfg, ).Get( context.Background(), testProjectNamespace, From 13702d5bb8980eef95aa1a8bad22e8b9bf45bd54 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Fri, 21 Feb 2025 22:25:16 +0100 Subject: [PATCH 2/2] feat(chart): allow configuration of HTTP credentials Signed-off-by: Hidde Beydals --- charts/kargo/README.md | 1 + charts/kargo/templates/controller/configmap.yaml | 1 + charts/kargo/values.yaml | 3 +++ 3 files changed, 5 insertions(+) diff --git a/charts/kargo/README.md b/charts/kargo/README.md index 6f1c9a1e3..c028c5be4 100644 --- a/charts/kargo/README.md +++ b/charts/kargo/README.md @@ -148,6 +148,7 @@ the Kargo controller is running. | `controller.serviceAccount.iamRole` | Specifies the ARN of an AWS IAM role to be used by the controller in an IRSA-enabled EKS cluster. | `""` | | `controller.serviceAccount.clusterWideSecretReadingEnabled` | Specifies whether the controller's ServiceAccount should be granted read permissions to Secrets CLUSTER-WIDE in the Kargo control plane's cluster. Enabling this is highly discouraged and you do so at your own peril. When this is NOT enabled, the Kargo management controller will dynamically expand and contract the controller's permissions to read Secrets on a Project-by-Project basis. | `false` | | `controller.globalCredentials.namespaces` | List of namespaces to look for shared credentials. Note that as of v1.0.0, the Kargo controller does not have cluster-wide access to Secrets. The controller receives read-only permission for Secrets on a per-Project basis as Projects are created. If you designate some namespaces as homes for "global" credentials, you will need to manually grant the controller permission to read Secrets in those namespaces. | `[]` | +| `controller.allowCredentialsOverHTTP` | Specifies whether the controller should allow credentials (for Git repositories, etc.) to be retrieved and used for operations over HTTP. This is generally discouraged, as it can expose sensitive information. When set to `false`, the controller will only allow credentials to be used over HTTPS (or other secure protocols). | `false` | | `controller.reconcilers.maxConcurrentReconciles` | specifies the maximum number of resources EACH of the controller's reconcilers can reconcile concurrently. This setting may also be overridden on a per-reconciler basis. | `4` | | `controller.reconcilers.controlFlowStages.maxConcurrentReconciles` | optionally overrides the maximum number of control flow Stage resources the controller can reconcile concurrently. | `nil` | | `controller.reconcilers.promotions.maxConcurrentReconciles` | optionally overrides the maximum number of Promotion resources the controller can reconcile concurrently. | `nil` | diff --git a/charts/kargo/templates/controller/configmap.yaml b/charts/kargo/templates/controller/configmap.yaml index e38a812a5..6312d73fb 100644 --- a/charts/kargo/templates/controller/configmap.yaml +++ b/charts/kargo/templates/controller/configmap.yaml @@ -19,6 +19,7 @@ data: KUBECONFIG: /etc/kargo/kubeconfigs/kubeconfig.yaml {{- end }} GLOBAL_CREDENTIALS_NAMESPACES: {{ quote (join "," .Values.controller.globalCredentials.namespaces) }} + ALLOW_CREDENTIALS_OVER_HTTP: {{ quote .Values.controller.allowCredentialsOverHTTP }} GITCLIENT_NAME: {{ quote .Values.controller.gitClient.name }} GITCLIENT_EMAIL: {{ quote .Values.controller.gitClient.email }} GITCLIENT_SIGNING_KEY_TYPE: {{ .Values.controller.gitClient.signingKeySecret.type | default "gpg" | quote }} diff --git a/charts/kargo/values.yaml b/charts/kargo/values.yaml index 4c85ccd09..96a7d7c14 100755 --- a/charts/kargo/values.yaml +++ b/charts/kargo/values.yaml @@ -379,6 +379,9 @@ controller: ## @param controller.globalCredentials.namespaces List of namespaces to look for shared credentials. Note that as of v1.0.0, the Kargo controller does not have cluster-wide access to Secrets. The controller receives read-only permission for Secrets on a per-Project basis as Projects are created. If you designate some namespaces as homes for "global" credentials, you will need to manually grant the controller permission to read Secrets in those namespaces. namespaces: [] + ## @param controller.allowCredentialsOverHTTP Specifies whether the controller should allow credentials (for Git repositories, etc.) to be retrieved and used for operations over HTTP. This is generally discouraged, as it can expose sensitive information. When set to `false`, the controller will only allow credentials to be used over HTTPS (or other secure protocols). + allowCredentialsOverHTTP: false + ## Reconciler-specific settings reconcilers: ## @param controller.reconcilers.maxConcurrentReconciles specifies the maximum number of resources EACH of the controller's reconcilers can reconcile concurrently. This setting may also be overridden on a per-reconciler basis.