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

feat: allow opt-in to credentials over HTTP #3539

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions charts/kargo/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down
1 change: 1 addition & 0 deletions charts/kargo/templates/controller/configmap.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }}
Expand Down
3 changes: 3 additions & 0 deletions charts/kargo/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion internal/credentials/kubernetes/database.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
47 changes: 36 additions & 11 deletions internal/credentials/kubernetes/database_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -262,6 +275,9 @@ func TestGet(t *testing.T) {
globalGitCredentialWithRepoURL,
globalGitCredentialWithRepoURLPattern,
},
cfg: DatabaseConfig{
GlobalCredentialsNamespaces: []string{testGlobalNamespace},
},
credType: credentials.TypeGit,
repoURL: testGitRepoURL,
expected: globalGitCredentialWithRepoURL,
Expand Down Expand Up @@ -296,16 +312,25 @@ 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 {
t.Run(testCase.name, func(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,
Expand Down
Loading