From 62c86b51e7cff895a7bd86c910fe6d1800afb389 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 15 Sep 2023 14:51:07 -0400 Subject: [PATCH 1/5] add TLS configuration options to API server Signed-off-by: Kent --- internal/api/config/config.go | 80 +++++++++++++++++++++-------------- 1 file changed, 48 insertions(+), 32 deletions(-) diff --git a/internal/api/config/config.go b/internal/api/config/config.go index 2e06d257b..d465506b8 100644 --- a/internal/api/config/config.go +++ b/internal/api/config/config.go @@ -13,61 +13,77 @@ import ( "github.com/akuity/kargo/internal/types" ) -// AdminConfig represents configuration for an admin account. -type AdminConfig struct { - // HashedPassword is a bcrypt hash of the password for the admin account. - HashedPassword string `envconfig:"ADMIN_ACCOUNT_PASSWORD_HASH" required:"true"` - // TokenIssuer is the value to be used in the ISS claim of ID tokens issued for - // the admin account. - TokenIssuer string `envconfig:"ADMIN_ACCOUNT_TOKEN_ISSUER" required:"true"` - // TokenAudience is the value to be used in the AUD claim of ID tokens issued - // for the admin account. - TokenAudience string `envconfig:"ADMIN_ACCOUNT_TOKEN_AUDIENCE" required:"true"` - // TokenSigningKey is the key used to sign ID tokens for the admin account. - TokenSigningKey []byte `envconfig:"ADMIN_ACCOUNT_TOKEN_SIGNING_KEY" required:"true"` - // TokenTTL specifies how long ID tokens for the admin account are valid. i.e. - // The expiry will be the time of issue plus this duration. - TokenTTL time.Duration `envconfig:"ADMIN_ACCOUNT_TOKEN_TTL" default:"1h"` -} - -// AdminConfigFromEnv returns an AdminConfig populated from environment -// variables. -func AdminConfigFromEnv() AdminConfig { - cfg := AdminConfig{} - envconfig.MustProcess("", &cfg) - return cfg +type StandardConfig struct { + GracefulShutdownTimeout time.Duration `envconfig:"GRACEFUL_SHUTDOWN_TIMEOUT" default:"30s"` + UIDirectory string `envconfig:"UI_DIR" default:"./ui/build"` } type ServerConfig struct { StandardConfig LocalMode bool + TLSConfig *TLSConfig OIDCConfig *oidc.Config AdminConfig *AdminConfig DexProxyConfig *dex.ProxyConfig ArgoCDConfig ArgoCDConfig } -type StandardConfig struct { - GracefulShutdownTimeout time.Duration `envconfig:"GRACEFUL_SHUTDOWN_TIMEOUT" default:"30s"` - UIDirectory string `envconfig:"UI_DIR" default:"./ui/build"` -} - func ServerConfigFromEnv() ServerConfig { cfg := ServerConfig{} envconfig.MustProcess("", &cfg.StandardConfig) - envconfig.MustProcess("", &cfg.ArgoCDConfig) - if types.MustParseBool(os.GetEnv("ADMIN_ACCOUNT_ENABLED", "false")) { - adminCfg := AdminConfigFromEnv() - cfg.AdminConfig = &adminCfg + if types.MustParseBool(os.GetEnv("TLS_ENABLED", "false")) { + tlsCfg := TLSConfigFromEnv() + cfg.TLSConfig = &tlsCfg } if types.MustParseBool(os.GetEnv("OIDC_ENABLED", "false")) { oidcCfg := oidc.ConfigFromEnv() cfg.OIDCConfig = &oidcCfg } + if types.MustParseBool(os.GetEnv("ADMIN_ACCOUNT_ENABLED", "false")) { + adminCfg := AdminConfigFromEnv() + cfg.AdminConfig = &adminCfg + } if types.MustParseBool(os.GetEnv("DEX_ENABLED", "false")) { dexProxyCfg := dex.ProxyConfigFromEnv() cfg.DexProxyConfig = &dexProxyCfg } + envconfig.MustProcess("", &cfg.ArgoCDConfig) + return cfg +} + +type TLSConfig struct { + CertPath string `envconfig:"TLS_CERT_PATH" required:"true"` + KeyPath string `envconfig:"TLS_KEY_PATH" required:"true"` +} + +func TLSConfigFromEnv() TLSConfig { + cfg := TLSConfig{} + envconfig.MustProcess("", &cfg) + return cfg +} + +// AdminConfig represents configuration for an admin account. +type AdminConfig struct { + // HashedPassword is a bcrypt hash of the password for the admin account. + HashedPassword string `envconfig:"ADMIN_ACCOUNT_PASSWORD_HASH" required:"true"` + // TokenIssuer is the value to be used in the ISS claim of ID tokens issued for + // the admin account. + TokenIssuer string `envconfig:"ADMIN_ACCOUNT_TOKEN_ISSUER" required:"true"` + // TokenAudience is the value to be used in the AUD claim of ID tokens issued + // for the admin account. + TokenAudience string `envconfig:"ADMIN_ACCOUNT_TOKEN_AUDIENCE" required:"true"` + // TokenSigningKey is the key used to sign ID tokens for the admin account. + TokenSigningKey []byte `envconfig:"ADMIN_ACCOUNT_TOKEN_SIGNING_KEY" required:"true"` + // TokenTTL specifies how long ID tokens for the admin account are valid. i.e. + // The expiry will be the time of issue plus this duration. + TokenTTL time.Duration `envconfig:"ADMIN_ACCOUNT_TOKEN_TTL" default:"1h"` +} + +// AdminConfigFromEnv returns an AdminConfig populated from environment +// variables. +func AdminConfigFromEnv() AdminConfig { + cfg := AdminConfig{} + envconfig.MustProcess("", &cfg) return cfg } From 572fbb49bf02b456b5646168492b4010ee740558 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 15 Sep 2023 14:56:24 -0400 Subject: [PATCH 2/5] add ability for API server to run with TLS Signed-off-by: Kent --- internal/api/server.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/internal/api/server.go b/internal/api/server.go index cce49b2cd..c0cc7d1f4 100644 --- a/internal/api/server.go +++ b/internal/api/server.go @@ -9,6 +9,7 @@ import ( "connectrpc.com/grpchealth" "github.com/pkg/errors" + "github.com/sirupsen/logrus" "golang.org/x/net/http2" "golang.org/x/net/http2/h2c" @@ -76,9 +77,21 @@ func (s *server) Serve(ctx context.Context, l net.Listener) error { } errCh := make(chan error) - go func() { errCh <- srv.Serve(l) }() + go func() { + if s.cfg.TLSConfig != nil { + errCh <- srv.ServeTLS( + l, + s.cfg.TLSConfig.CertPath, + s.cfg.TLSConfig.KeyPath, + ) + } else { + errCh <- srv.Serve(l) + } + }() - log.Infof("Server is listening on %q", l.Addr().String()) + log.WithFields(logrus.Fields{ + "tls": s.cfg.TLSConfig != nil, + }).Infof("Server is listening on %q", l.Addr().String()) select { case <-ctx.Done(): From 4fd640444f99d7e1a36fba93fa16049f036c1355 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 15 Sep 2023 16:44:50 -0400 Subject: [PATCH 3/5] corresponding chart changes Signed-off-by: Kent --- charts/kargo/templates/api/cert.yaml | 17 +++++++++ charts/kargo/templates/api/configmap.yaml | 17 +++++++-- charts/kargo/templates/api/deployment.yaml | 27 ++++++++++++-- charts/kargo/templates/api/ingress.yaml | 4 +++ charts/kargo/templates/api/service.yaml | 4 +++ charts/kargo/templates/dex-server/secret.yaml | 12 +++++-- charts/kargo/values.yaml | 36 +++++++++++++------ 7 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 charts/kargo/templates/api/cert.yaml diff --git a/charts/kargo/templates/api/cert.yaml b/charts/kargo/templates/api/cert.yaml new file mode 100644 index 000000000..6b1c1ebcd --- /dev/null +++ b/charts/kargo/templates/api/cert.yaml @@ -0,0 +1,17 @@ +{{- if and .Values.api.enabled .Values.api.tls.enabled .Values.api.tls.selfSignedCert }} +apiVersion: cert-manager.io/v1 +kind: Certificate +metadata: + name: kargo-api + namespace: {{ .Release.Namespace }} + labels: + {{- include "kargo.labels" . | nindent 4 }} + {{- include "kargo.api.labels" . | nindent 4 }} +spec: + dnsNames: + - {{ .Values.api.host }} + issuerRef: + kind: Issuer + name: kargo-selfsigned-cert-issuer + secretName: kargo-api-cert +{{- end }} diff --git a/charts/kargo/templates/api/configmap.yaml b/charts/kargo/templates/api/configmap.yaml index 01a294f13..e11b30abf 100644 --- a/charts/kargo/templates/api/configmap.yaml +++ b/charts/kargo/templates/api/configmap.yaml @@ -12,16 +12,29 @@ data: {{- if .Values.kubeconfigSecrets.kargo }} KUBECONFIG: /etc/kargo/kubeconfig.yaml {{- end }} + {{- if .Values.api.tls.enabled }} + TLS_ENABLED: "true" + TLS_CERT_PATH: /etc/kargo/tls.crt + TLS_KEY_PATH: /etc/kargo/tls.key + {{- end }} {{- if .Values.api.adminAccount.enabled }} ADMIN_ACCOUNT_ENABLED: "true" - ADMIN_ACCOUNT_TOKEN_ISSUER: {{ .Values.api.protocol }}://{{ .Values.api.host }} + {{- if or .Values.api.tls.enabled (and .Values.api.ingress.enabled .Values.api.ingress.tls.enabled) }} + ADMIN_ACCOUNT_TOKEN_ISSUER: https://{{ .Values.api.host }} + {{- else }} + ADMIN_ACCOUNT_TOKEN_ISSUER: http://{{ .Values.api.host }} + {{- end }} ADMIN_ACCOUNT_TOKEN_AUDIENCE: {{ .Values.api.host }} ADMIN_ACCOUNT_TOKEN_TTL: {{ .Values.api.adminAccount.tokenTTL }} {{- end }} {{- if .Values.api.oidc.enabled }} OIDC_ENABLED: "true" {{- if .Values.api.oidc.dex.enabled }} - OIDC_ISSUER_URL: {{ .Values.api.protocol }}://{{ .Values.api.host }}/dex + {{- if or .Values.api.tls.enabled (and .Values.api.ingress.enabled .Values.api.ingress.tls.enabled) }} + OIDC_ISSUER_URL: https://{{ .Values.api.host }}/dex + {{- else }} + OIDC_ISSUER_URL: http://{{ .Values.api.host }}/dex + {{- end }} OIDC_CLIENT_ID: {{ .Values.api.host }} OIDC_CLI_CLIENT_ID: {{ .Values.api.host }}-cli DEX_ENABLED: "true" diff --git a/charts/kargo/templates/api/deployment.yaml b/charts/kargo/templates/api/deployment.yaml index c0f01c102..993fb53e8 100644 --- a/charts/kargo/templates/api/deployment.yaml +++ b/charts/kargo/templates/api/deployment.yaml @@ -41,11 +41,23 @@ spec: protocol: TCP livenessProbe: exec: - command: ["/usr/local/bin/grpc_health_probe", "-addr=:8080"] + command: + - /usr/local/bin/grpc_health_probe + - -addr=:8080 +{{- if .Values.api.tls.enabled }} + - -tls + - -tls-no-verify +{{- end }} initialDelaySeconds: 10 readinessProbe: exec: - command: ["/usr/local/bin/grpc_health_probe", "-addr=:8080"] + command: + - /usr/local/bin/grpc_health_probe + - -addr=:8080 +{{- if .Values.api.tls.enabled }} + - -tls + - -tls-no-verify +{{- end }} initialDelaySeconds: 5 {{- if or .Values.kubeconfigSecrets.kargo (and .Values.api.oidc.enabled .Values.api.oidc.dex.enabled) }} volumeMounts: @@ -54,7 +66,7 @@ spec: readOnly: true {{- end }} resources: {{ toYaml .Values.api.resources }} -{{- if or .Values.kubeconfigSecrets.kargo (and .Values.api.oidc.enabled .Values.api.oidc.dex.enabled) }} +{{- if or .Values.kubeconfigSecrets.kargo (and .Values.api.oidc.enabled .Values.api.oidc.dex.enabled) .Values.api.tls.enabled }} volumes: - name: config projected: @@ -67,6 +79,15 @@ spec: path: kubeconfig.yaml mode: 0644 {{- end }} +{{- if .Values.api.tls.enabled }} + - secret: + name: kargo-api-cert + items: + - key: tls.crt + path: tls.crt + - key: tls.key + path: tls.key +{{- end }} {{- if and .Values.api.oidc.enabled .Values.api.oidc.dex.enabled }} - secret: name: kargo-dex-server-cert diff --git a/charts/kargo/templates/api/ingress.yaml b/charts/kargo/templates/api/ingress.yaml index e3180b9e6..e4fb2efe4 100644 --- a/charts/kargo/templates/api/ingress.yaml +++ b/charts/kargo/templates/api/ingress.yaml @@ -25,7 +25,11 @@ spec: service: name: kargo-api port: + {{- if .Values.api.tls.enabled }} + number: 443 + {{- else }} number: 80 + {{- end }} {{- if .Values.api.ingress.tls.enabled }} tls: - hosts: diff --git a/charts/kargo/templates/api/service.yaml b/charts/kargo/templates/api/service.yaml index f9fc382b9..6a0dda13e 100644 --- a/charts/kargo/templates/api/service.yaml +++ b/charts/kargo/templates/api/service.yaml @@ -11,7 +11,11 @@ spec: type: {{ .Values.api.service.type }} ports: - protocol: TCP + {{- if .Values.api.tls.enabled }} + port: 443 + {{- else }} port: 80 + {{- end }} {{- if and (or (eq .Values.api.service.type "NodePort") (eq .Values.api.service.type "LoadBalancer")) .Values.api.service.nodePort}} nodePort: {{ .Values.api.service.nodePort }} {{- end }} diff --git a/charts/kargo/templates/dex-server/secret.yaml b/charts/kargo/templates/dex-server/secret.yaml index 52fc34a70..62775098b 100644 --- a/charts/kargo/templates/dex-server/secret.yaml +++ b/charts/kargo/templates/dex-server/secret.yaml @@ -10,7 +10,11 @@ metadata: {{- include "kargo.dexServer.labels" . | nindent 4 }} stringData: config.yaml: |- - issuer: {{ .Values.api.protocol }}://{{ .Values.api.host }}/dex + {{- if or .Values.api.tls.enabled (and .Values.api.ingress.enabled .Values.api.ingress.tls.enabled) }} + issuer: https://{{ .Values.api.host }}/dex + {{- else }} + issuer: http://{{ .Values.api.host }}/dex + {{- end }} storage: type: memory @@ -30,7 +34,11 @@ stringData: name: Kargo public: true redirectURIs: - - {{ .Values.api.protocol }}://{{ .Values.api.host }}/login + {{- if or .Values.api.tls.enabled (and .Values.api.ingress.enabled .Values.api.ingress.tls.enabled) }} + - https://{{ .Values.api.host }}/login + {{- else }} + - http://{{ .Values.api.host }}/login + {{- end }} - id: {{ .Values.api.host }}-cli name: Kargo CLI public: true diff --git a/charts/kargo/values.yaml b/charts/kargo/values.yaml index a52bf3d2f..62e34283a 100755 --- a/charts/kargo/values.yaml +++ b/charts/kargo/values.yaml @@ -48,15 +48,33 @@ api: ## The number of API server pods. (Default: 1) # replicas: 3 - ## The protocol and domain name where Kargo's API server will be accessible. - ## This should be set accurately for a variety of reasons, including (when - ## applicable) generation of a correct Ingress resource and correct OpenID - ## Connect issuer and callback URLs. - protocol: http + ## The domain name where Kargo's API server will be accessible. This should be + ## set accurately for a variety of reasons, including (when applicable) + ## generation of a correct Ingress resource and correct OpenID Connect issuer + ## and callback URLs. + ## + ## Note: The protocol (http vs https) should not be specified and is + ## automatically inferred by from other configuration options. host: localhost logLevel: INFO + tls: + ## Whether to enable TLS directly on the API server. This is helpful if you + ## do not intend to use an Ingress controller OR you require TLS end-to-end. + ## + ## All other settings in this section will be ignored when this is set to + ## false. + enabled: true + ## Whether to generate a self-signed certificate for use by the API server. + ## + ## If true, cert-manager CRDs MUST be pre-installed on this cluster. + ## Kargo will create and use its own namespaced issuer. + ## + ## If false, a cert secret named + ## kargo-api-cert MUST be provided in the same namespace as Kargo. + selfSignedCert: true + ingress: ## Whether to enable ingress. By default, this is disabled. Enabling ingress ## is advanced usage. @@ -73,7 +91,7 @@ api: ## class as annotation. ingressClassName: tls: - ## Whether to enable TLS. + ## Whether to enable TLS on the Ingress. ## ## All other settings in this section will be ignored when this is set to ## false. @@ -86,8 +104,6 @@ api: ## ## If false, a cert secret named ## kargo-api-ingress-cert MUST be provided in the same namespace as Kargo. - ## - ## There is no provision for running Dex without TLS. selfSignedCert: true service: @@ -214,7 +230,7 @@ api: # config: # clientID: # clientSecret: - # redirectURI: :///dex/callback + # redirectURI: :///dex/callback ## GitHub Example # - id: github # name: GitHub @@ -222,7 +238,7 @@ api: # config: # clientID: # clientSecret: - # redirectURI: :///dex/callback + # redirectURI: :///dex/callback resources: {} # We usually recommend not to specify default resources and to leave From 15f44f44565a6c553789171fbc76c88018b4f984 Mon Sep 17 00:00:00 2001 From: Kent Date: Fri, 15 Sep 2023 16:45:25 -0400 Subject: [PATCH 4/5] corresponding Tiltfile changes Signed-off-by: Kent --- Tiltfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tiltfile b/Tiltfile index 1c9abd2bf..15fa48c4e 100644 --- a/Tiltfile +++ b/Tiltfile @@ -47,6 +47,7 @@ k8s_resource( ], labels = ['kargo'], objects = [ + 'kargo-api:certificate', 'kargo-api:clusterrole', 'kargo-api:clusterrolebinding', 'kargo-api:configmap', @@ -128,7 +129,6 @@ k8s_yaml( namespace = 'kargo', set = [ 'api.logLevel=DEBUG', - 'api.protocol=http', 'api.host=localhost:30081', 'api.adminAccount.password=admin', 'api.adminAccount.tokenSigningKey=iwishtowashmyirishwristwatch', From 653444fa27444d85f44d68f3d97e20b46465a0f6 Mon Sep 17 00:00:00 2001 From: Kent Rancourt Date: Sat, 16 Sep 2023 10:52:16 -0400 Subject: [PATCH 5/5] Update internal/api/config/config.go Co-authored-by: Sunghoon Kang --- internal/api/config/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/api/config/config.go b/internal/api/config/config.go index d465506b8..ca79a2467 100644 --- a/internal/api/config/config.go +++ b/internal/api/config/config.go @@ -82,7 +82,7 @@ type AdminConfig struct { // AdminConfigFromEnv returns an AdminConfig populated from environment // variables. func AdminConfigFromEnv() AdminConfig { - cfg := AdminConfig{} + var cfg AdminConfig envconfig.MustProcess("", &cfg) return cfg }