From 7f9cc1789169c15d57da5d4d749db60e3b4d27fd Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:37:50 +0300 Subject: [PATCH 1/9] fix(instance): pass database URL to the init script via env spec.databaseURL was interpolated straight into the sh -c init script, so a value containing quotes, $(...) or backticks ran arbitrary commands in the init container. Pass it as DATABASE_URL and reference "${DATABASE_URL}" in the script, the same way ADMIN_PASSWORD is handled. --- .../controller/warpgateinstance_controller.go | 35 ++++++----- .../warpgateinstance_controller_test.go | 63 ++++++++++++++++++- openspec/specs/instance-management/spec.md | 10 +++ 3 files changed, 92 insertions(+), 16 deletions(-) diff --git a/internal/controller/warpgateinstance_controller.go b/internal/controller/warpgateinstance_controller.go index ad0989a..67ce778 100644 --- a/internal/controller/warpgateinstance_controller.go +++ b/internal/controller/warpgateinstance_controller.go @@ -554,7 +554,9 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp setupCmd += fmt.Sprintf(` --kubernetes-port %d`, instanceKubernetesPort(inst)) } if inst.Spec.DatabaseURL != "" { - setupCmd += fmt.Sprintf(` --database-url "%s"`, inst.Spec.DatabaseURL) + // The URL reaches the script through the environment so the shell never + // parses it as code. + setupCmd += ` --database-url "${DATABASE_URL}"` } if inst.Spec.RecordSessions != nil && *inst.Spec.RecordSessions { // ponytail: seeded at first setup only; later toggles need the admin UI or parameters API @@ -610,6 +612,23 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp initScript := strings.Join(scriptParts, "\n") + initEnv := []corev1.EnvVar{ + { + Name: "ADMIN_PASSWORD", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: inst.Spec.AdminPasswordSecretRef.Name, + }, + Key: adminPasswordKey(inst), + }, + }, + }, + } + if inst.Spec.DatabaseURL != "" { + initEnv = append(initEnv, corev1.EnvVar{Name: "DATABASE_URL", Value: inst.Spec.DatabaseURL}) + } + // Init container volume mounts initVolumeMounts := []corev1.VolumeMount{ {Name: "data", MountPath: "/data"}, @@ -775,19 +794,7 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp Image: image, Command: []string{"/bin/sh", "-c", initScript}, VolumeMounts: initVolumeMounts, - Env: []corev1.EnvVar{ - { - Name: "ADMIN_PASSWORD", - ValueFrom: &corev1.EnvVarSource{ - SecretKeyRef: &corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{ - Name: inst.Spec.AdminPasswordSecretRef.Name, - }, - Key: adminPasswordKey(inst), - }, - }, - }, - }, + Env: initEnv, }, }, Containers: []corev1.Container{ diff --git a/internal/controller/warpgateinstance_controller_test.go b/internal/controller/warpgateinstance_controller_test.go index 97dc5f8..e4dcb2a 100644 --- a/internal/controller/warpgateinstance_controller_test.go +++ b/internal/controller/warpgateinstance_controller_test.go @@ -3485,8 +3485,67 @@ var _ = Describe("WarpgateInstance Controller", func() { } deploy := reconciler.buildDeployment(inst) initCmd := deploy.Spec.Template.Spec.InitContainers[0].Command[2] - Expect(initCmd).To(ContainSubstring("--database-url")) - Expect(initCmd).To(ContainSubstring("postgres://host/db")) + Expect(initCmd).To(ContainSubstring(`--database-url "${DATABASE_URL}"`)) + Expect(initCmd).NotTo(ContainSubstring("postgres://host/db")) + Expect(deploy.Spec.Template.Spec.InitContainers[0].Env).To(ContainElement( + corev1.EnvVar{Name: "DATABASE_URL", Value: "postgres://host/db"}, + )) + }) + + It("should never put a hostile DatabaseURL into the init script", func() { + hostile := "postgres://u:p@h/db\"; touch /pwned; echo \"$(id)`whoami`${HOME}'" + inst := &warpgatev1alpha1.WarpgateInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deploy-dburl-inject", + Namespace: testNamespace, + }, + Spec: warpgatev1alpha1.WarpgateInstanceSpec{ + Version: "0.21.1", + AdminPasswordSecretRef: warpgatev1alpha1.SecretKeyRef{ + Name: "dummy", + }, + HTTP: &warpgatev1alpha1.HTTPListenerSpec{ + Enabled: boolPtr(true), + }, + TLS: &warpgatev1alpha1.InstanceTLSSpec{ + CertManager: boolPtr(false), + }, + DatabaseURL: hostile, + }, + } + deploy := reconciler.buildDeployment(inst) + initCtr := deploy.Spec.Template.Spec.InitContainers[0] + script := initCtr.Command[2] + + for _, fragment := range []string{"touch /pwned", "$(id)", "`whoami`", "${HOME}", "u:p@h"} { + Expect(script).NotTo(ContainSubstring(fragment)) + } + Expect(script).To(ContainSubstring(`--database-url "${DATABASE_URL}"`)) + Expect(initCtr.Env).To(ContainElement(corev1.EnvVar{Name: "DATABASE_URL", Value: hostile})) + }) + + It("should not set DATABASE_URL when DatabaseURL is empty", func() { + inst := &warpgatev1alpha1.WarpgateInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deploy-no-dburl", + Namespace: testNamespace, + }, + Spec: warpgatev1alpha1.WarpgateInstanceSpec{ + Version: "0.21.1", + AdminPasswordSecretRef: warpgatev1alpha1.SecretKeyRef{ + Name: "dummy", + }, + TLS: &warpgatev1alpha1.InstanceTLSSpec{ + CertManager: boolPtr(false), + }, + }, + } + deploy := reconciler.buildDeployment(inst) + initCtr := deploy.Spec.Template.Spec.InitContainers[0] + Expect(initCtr.Command[2]).NotTo(ContainSubstring("--database-url")) + for _, e := range initCtr.Env { + Expect(e.Name).NotTo(Equal("DATABASE_URL")) + } }) It("should use emptyDir volume when storage is disabled", func() { diff --git a/openspec/specs/instance-management/spec.md b/openspec/specs/instance-management/spec.md index 5742808..6288505 100644 --- a/openspec/specs/instance-management/spec.md +++ b/openspec/specs/instance-management/spec.md @@ -103,3 +103,13 @@ The controller requeues every 5 minutes. On each pass it compares the owned reso - **Given** a deployed `WarpgateInstance` whose StatefulSet was manually edited **When** the controller reconciles **Then** it overwrites the StatefulSet back to the desired state. - **Given** a deployed `WarpgateInstance` whose HTTP Service was deleted **When** the controller reconciles **Then** it recreates the Service. + +### REQ-INST-007: No Spec Values in the Init Script + +**Status:** ADDED + +The init container runs a generated `sh -c` script. User-controlled values (admin password, database URL) are passed to it as environment variables and referenced as quoted expansions (`"${ADMIN_PASSWORD}"`, `"${DATABASE_URL}"`), so the shell never parses them as code. + +**Scenarios:** + +- **Given** a `WarpgateInstance` whose `databaseURL` contains shell metacharacters (`"`, `;`, `$(...)`, backticks) **When** the controller builds the Deployment **Then** the init script only references `${DATABASE_URL}` and the raw value appears solely in the init container's `DATABASE_URL` environment variable. From e662340b7282aa94b70321e32ec4ee3b4375dbc1 Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:42:24 +0300 Subject: [PATCH 2/9] fix(instance): keep database credentials out of the ConfigMap The external database URL (usually with a password in it) was written in plain text into the generated warpgate.yaml ConfigMap. Warpgate has no env override for database_url, so the ConfigMap now leaves it out and the init container appends it to /data/warpgate.yaml from DATABASE_URL, quoted as a YAML single-quoted scalar. Add spec.databaseURLSecretRef so the URL can come from a Secret. It takes precedence over spec.databaseURL, which keeps working but is now deprecated and gets an admission warning. externalHost is emitted through a YAML marshaller instead of printf, and the init script is folded into the pod template hash so existing Deployments roll onto the new script. --- api/v1alpha1/warpgateinstance_types.go | 7 ++ api/v1alpha1/warpgateinstance_webhook.go | 31 ++++- api/v1alpha1/warpgateinstance_webhook_test.go | 70 ++++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 5 + .../templates/crds/warpgateinstances.yaml | 18 +++ ....warpgate.warp.tech_warpgateinstances.yaml | 18 +++ docs/crds/warpgate-instance.md | 9 ++ go.mod | 2 +- .../controller/warpgateinstance_controller.go | 72 ++++++++++-- .../warpgateinstance_controller_test.go | 107 +++++++++++++++++- openspec/specs/instance-management/spec.md | 15 +++ 11 files changed, 337 insertions(+), 17 deletions(-) diff --git a/api/v1alpha1/warpgateinstance_types.go b/api/v1alpha1/warpgateinstance_types.go index 0c3d818..bbef5cb 100644 --- a/api/v1alpha1/warpgateinstance_types.go +++ b/api/v1alpha1/warpgateinstance_types.go @@ -82,9 +82,16 @@ type WarpgateInstanceSpec struct { // databaseURL overrides the default SQLite database with a PostgreSQL connection string. // Example: "postgres://user:pass@host:5432/warpgate" + // This field is deprecated because the value (including any password) is stored in + // plain text in this resource and in the generated Deployment. Use databaseURLSecretRef. // +optional DatabaseURL string `json:"databaseURL,omitempty"` + // databaseURLSecretRef references a Secret key holding the database connection string. + // Takes precedence over databaseURL. The key defaults to "url". + // +optional + DatabaseURLSecretRef *SecretKeyRef `json:"databaseURLSecretRef,omitempty"` + // recordSessions enables session recording. Defaults to false. // +optional RecordSessions *bool `json:"recordSessions,omitempty"` diff --git a/api/v1alpha1/warpgateinstance_webhook.go b/api/v1alpha1/warpgateinstance_webhook.go index 3fc4be0..31d7767 100644 --- a/api/v1alpha1/warpgateinstance_webhook.go +++ b/api/v1alpha1/warpgateinstance_webhook.go @@ -197,6 +197,28 @@ func validatePort(port *int32, fldPath *field.Path, allowZero bool) *field.Error return nil } +// validateDatabase checks the external database settings and warns about the +// plain-text databaseURL field. +func validateDatabase(inst *WarpgateInstance, specPath *field.Path) (field.ErrorList, admission.Warnings) { + var errs field.ErrorList + var warnings admission.Warnings + dbRef := inst.Spec.DatabaseURLSecretRef + if dbRef != nil && dbRef.Name == "" { + errs = append(errs, field.Required(specPath.Child("databaseURLSecretRef", "name"), "databaseURLSecretRef.name must not be empty")) + } + if inst.Spec.DatabaseURL != "" || dbRef != nil { + warnings = append(warnings, "an external database is configured; SQLite persistence via PVC is not needed") + } + if inst.Spec.DatabaseURL != "" { + if dbRef != nil { + warnings = append(warnings, "databaseURL is ignored because databaseURLSecretRef is set") + } else { + warnings = append(warnings, "databaseURL is deprecated and stored in plain text; move it to a Secret and use databaseURLSecretRef") + } + } + return errs, warnings +} + // validateWarpgateInstance runs all field-level validation checks. func validateWarpgateInstance(inst *WarpgateInstance) (admission.Warnings, error) { var allErrs field.ErrorList @@ -253,11 +275,12 @@ func validateWarpgateInstance(inst *WarpgateInstance) (admission.Warnings, error } } - if inst.Spec.DatabaseURL != "" { - warnings = append(warnings, "databaseURL is set — SQLite persistence via PVC is not needed") - } + dbErrs, dbWarnings := validateDatabase(inst, specPath) + allErrs = append(allErrs, dbErrs...) + warnings = append(warnings, dbWarnings...) + externalDB := inst.Spec.DatabaseURL != "" || inst.Spec.DatabaseURLSecretRef != nil noStorage := inst.Spec.Storage != nil && inst.Spec.Storage.Enabled != nil && !*inst.Spec.Storage.Enabled - if noStorage && inst.Spec.DatabaseURL == "" { + if noStorage && !externalDB { warnings = append(warnings, "storage is disabled and no databaseURL is set — data will be lost on pod restart") } diff --git a/api/v1alpha1/warpgateinstance_webhook_test.go b/api/v1alpha1/warpgateinstance_webhook_test.go index c6ff851..21e5903 100644 --- a/api/v1alpha1/warpgateinstance_webhook_test.go +++ b/api/v1alpha1/warpgateinstance_webhook_test.go @@ -623,3 +623,73 @@ func TestInstanceValidate_NoWarnStorageDisabledWithDB(t *testing.T) { } } } + +func TestInstanceValidate_WarnDatabaseURLDeprecated(t *testing.T) { + inst := validInstance() + inst.Spec.DatabaseURL = "postgres://user:pass@host:5432/warpgate" + + warnings, err := (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !containsWarning(warnings, "deprecated") { + t.Errorf("expected a deprecation warning for databaseURL, got: %v", warnings) + } +} + +func TestInstanceValidate_DatabaseURLSecretRef(t *testing.T) { + inst := validInstance() + inst.Spec.DatabaseURLSecretRef = &SecretKeyRef{Name: "wg-db"} + + warnings, err := (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if containsWarning(warnings, "deprecated") { + t.Errorf("did not expect a deprecation warning, got: %v", warnings) + } + if !containsWarning(warnings, "SQLite") { + t.Errorf("expected the external database warning, got: %v", warnings) + } + + inst.Spec.DatabaseURL = "postgres://inline" + warnings, err = (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !containsWarning(warnings, "ignored") { + t.Errorf("expected a precedence warning when both are set, got: %v", warnings) + } +} + +func TestInstanceValidate_DatabaseURLSecretRefRequiresName(t *testing.T) { + inst := validInstance() + inst.Spec.DatabaseURLSecretRef = &SecretKeyRef{Key: "url"} + + if _, err := (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst); err == nil { + t.Fatal("expected an error for databaseURLSecretRef without a name") + } +} + +func TestInstanceValidate_NoDataLossWarnWithDatabaseURLSecretRef(t *testing.T) { + inst := validInstance() + inst.Spec.Storage = &StorageSpec{Enabled: boolPtr(false), Size: "1Gi"} + inst.Spec.DatabaseURLSecretRef = &SecretKeyRef{Name: "wg-db"} + + warnings, err := (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if containsWarning(warnings, "data will be lost") { + t.Errorf("should not warn about data loss with an external database, got: %v", warnings) + } +} + +func containsWarning(warnings []string, substr string) bool { + for _, w := range warnings { + if strings.Contains(w, substr) { + return true + } + } + return false +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 4354c45..dc86128 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -784,6 +784,11 @@ func (in *WarpgateInstanceSpec) DeepCopyInto(out *WarpgateInstanceSpec) { *out = new(InstanceTLSSpec) (*in).DeepCopyInto(*out) } + if in.DatabaseURLSecretRef != nil { + in, out := &in.DatabaseURLSecretRef, &out.DatabaseURLSecretRef + *out = new(SecretKeyRef) + **out = **in + } if in.RecordSessions != nil { in, out := &in.RecordSessions, &out.RecordSessions *out = new(bool) diff --git a/charts/warpgate-operator/templates/crds/warpgateinstances.yaml b/charts/warpgate-operator/templates/crds/warpgateinstances.yaml index e86ad7b..7d0865a 100644 --- a/charts/warpgate-operator/templates/crds/warpgateinstances.yaml +++ b/charts/warpgate-operator/templates/crds/warpgateinstances.yaml @@ -86,7 +86,25 @@ spec: description: |- databaseURL overrides the default SQLite database with a PostgreSQL connection string. Example: "postgres://user:pass@host:5432/warpgate" + This field is deprecated because the value (including any password) is stored in + plain text in this resource and in the generated Deployment. Use databaseURLSecretRef. type: string + databaseURLSecretRef: + description: |- + databaseURLSecretRef references a Secret key holding the database connection string. + Takes precedence over databaseURL. The key defaults to "url". + properties: + key: + description: |- + key is the key within the Secret. Defaults to "password" for password refs. + Unused by tokenSecretRef (which always reads "username" and "password" keys). + type: string + name: + description: name is the name of the Secret. + type: string + required: + - name + type: object externalHost: description: externalHost sets the external hostname for Warpgate cookie domain and URL generation. diff --git a/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml b/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml index 155a5da..2e71b74 100644 --- a/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml +++ b/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml @@ -85,7 +85,25 @@ spec: description: |- databaseURL overrides the default SQLite database with a PostgreSQL connection string. Example: "postgres://user:pass@host:5432/warpgate" + This field is deprecated because the value (including any password) is stored in + plain text in this resource and in the generated Deployment. Use databaseURLSecretRef. type: string + databaseURLSecretRef: + description: |- + databaseURLSecretRef references a Secret key holding the database connection string. + Takes precedence over databaseURL. The key defaults to "url". + properties: + key: + description: |- + key is the key within the Secret. Defaults to "password" for password refs. + Unused by tokenSecretRef (which always reads "username" and "password" keys). + type: string + name: + description: name is the name of the Secret. + type: string + required: + - name + type: object externalHost: description: externalHost sets the external hostname for Warpgate cookie domain and URL generation. diff --git a/docs/crds/warpgate-instance.md b/docs/crds/warpgate-instance.md index c09cecc..8ab84b5 100644 --- a/docs/crds/warpgate-instance.md +++ b/docs/crds/warpgate-instance.md @@ -34,6 +34,9 @@ CRDs like `WarpgateRole` and `WarpgateUser` can reference the deployed instance | `tolerations` | `[]Toleration` | No | - | Scheduling tolerations | | `createConnection` | `bool` | No | `true` | Auto-create a `WarpgateConnection` CR pointing to this instance | | `externalHost` | `string` | No | - | External hostname for cookie domain and URL generation | +| `databaseURLSecretRef.name` | `string` | No | - | Secret holding the external (PostgreSQL) database URL. Takes precedence over `databaseURL` | +| `databaseURLSecretRef.key` | `string` | No | `url` | Key in that Secret | +| `databaseURL` | `string` | No | - | **Deprecated.** Inline database URL, stored in plain text in the CR and Deployment. Use `databaseURLSecretRef` | ## Status Fields @@ -172,4 +175,10 @@ spec: StatefulSet's `volumeClaimTemplates`. Scaling down to zero replicas does not delete the PVC -- data persists across restarts. - **Scale subresource:** The CRD exposes a scale subresource (`spec.replicas` / `status.readyReplicas`), so you can use `kubectl scale` or HPA with it. +- **External database:** Put the connection string in a Secret and reference it with `databaseURLSecretRef` + (for example `kubectl create secret generic warpgate-db --from-literal=url='postgres://user:pass@host:5432/warpgate'`). + The URL never lands in the generated ConfigMap: the init container receives it as `DATABASE_URL` from the Secret + and writes it into `/data/warpgate.yaml` on the data volume. Rotating the Secret takes effect on the next pod + restart (`kubectl rollout restart deployment/`). The older `databaseURL` field still works but is deprecated, + since its value is visible to anyone who can read the `WarpgateInstance` or its Deployment. - **Admin password Secret:** The Secret must exist in the same namespace as the `WarpgateInstance` CR. The operator reads it at reconciliation time and injects it into the Warpgate configuration. diff --git a/go.mod b/go.mod index 1f0d459..3078b99 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( k8s.io/apimachinery v0.37.0 k8s.io/client-go v0.37.0 sigs.k8s.io/controller-runtime v0.25.1 + sigs.k8s.io/yaml v1.6.0 ) require ( @@ -103,5 +104,4 @@ require ( sigs.k8s.io/json v0.0.0-20250730193827-2d320260d730 // indirect sigs.k8s.io/randfill v1.0.0 // indirect sigs.k8s.io/structured-merge-diff/v6 v6.4.2 // indirect - sigs.k8s.io/yaml v1.6.0 // indirect ) diff --git a/internal/controller/warpgateinstance_controller.go b/internal/controller/warpgateinstance_controller.go index 67ce778..d7293d6 100644 --- a/internal/controller/warpgateinstance_controller.go +++ b/internal/controller/warpgateinstance_controller.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/yaml" warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" ) @@ -306,6 +307,44 @@ func adminPasswordKey(inst *warpgatev1alpha1.WarpgateInstance) string { return "password" } +// databaseURLFromSecret reports whether the database URL comes from a Secret. +// databaseURLSecretRef wins over the deprecated inline databaseURL. +func databaseURLFromSecret(inst *warpgatev1alpha1.WarpgateInstance) bool { + return inst.Spec.DatabaseURLSecretRef != nil && inst.Spec.DatabaseURLSecretRef.Name != "" +} + +// externalDatabase reports whether Warpgate should use a database other than +// the bundled SQLite file. +func externalDatabase(inst *warpgatev1alpha1.WarpgateInstance) bool { + return databaseURLFromSecret(inst) || inst.Spec.DatabaseURL != "" +} + +func databaseURLKey(inst *warpgatev1alpha1.WarpgateInstance) string { + if inst.Spec.DatabaseURLSecretRef != nil && inst.Spec.DatabaseURLSecretRef.Key != "" { + return inst.Spec.DatabaseURLSecretRef.Key + } + return "url" +} + +// databaseURLEnv returns the DATABASE_URL variable for the init container, +// sourced from the Secret when one is referenced. +func databaseURLEnv(inst *warpgatev1alpha1.WarpgateInstance) corev1.EnvVar { + if databaseURLFromSecret(inst) { + return corev1.EnvVar{ + Name: "DATABASE_URL", + ValueFrom: &corev1.EnvVarSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{ + Name: inst.Spec.DatabaseURLSecretRef.Name, + }, + Key: databaseURLKey(inst), + }, + }, + } + } + return corev1.EnvVar{Name: "DATABASE_URL", Value: inst.Spec.DatabaseURL} +} + func instanceLabels(inst *warpgatev1alpha1.WarpgateInstance) map[string]string { return map[string]string{ "app.kubernetes.io/name": "warpgate", @@ -330,6 +369,9 @@ func configHash(inst *warpgatev1alpha1.WarpgateInstance) string { h.Write([]byte(inst.Spec.ExternalHost)) h.Write([]byte(inst.Spec.ConfigOverride)) h.Write([]byte(inst.Spec.DatabaseURL)) + if databaseURLFromSecret(inst) { + h.Write([]byte(inst.Spec.DatabaseURLSecretRef.Name + "/" + databaseURLKey(inst))) + } h.Write([]byte(inst.Spec.SSHKeysSecretName)) return fmt.Sprintf("%x", h.Sum(nil))[:12] } @@ -341,9 +383,9 @@ func configHash(inst *warpgatev1alpha1.WarpgateInstance) string { func (r *WarpgateInstanceReconciler) buildWarpgateConfig(inst *warpgatev1alpha1.WarpgateInstance) string { var b strings.Builder - if inst.Spec.DatabaseURL != "" { - fmt.Fprintf(&b, "database_url: \"%s\"\n", inst.Spec.DatabaseURL) - } else { + // An external database URL usually carries credentials, so it never goes + // into this ConfigMap. The init container appends it from DATABASE_URL. + if !externalDatabase(inst) { b.WriteString("database_url: sqlite:/data/db\n") } @@ -377,7 +419,11 @@ func (r *WarpgateInstanceReconciler) buildWarpgateConfig(inst *warpgatev1alpha1. } if inst.Spec.ExternalHost != "" { - fmt.Fprintf(&b, "external_host: %s\n", inst.Spec.ExternalHost) + // Marshal instead of printf so the value can't break out of its scalar. + out, err := yaml.Marshal(map[string]string{"external_host": inst.Spec.ExternalHost}) + if err == nil { + b.Write(out) + } } // Session recording lives in Warpgate's database parameters, not the config @@ -553,7 +599,7 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp if kubernetesEnabled(inst) { setupCmd += fmt.Sprintf(` --kubernetes-port %d`, instanceKubernetesPort(inst)) } - if inst.Spec.DatabaseURL != "" { + if externalDatabase(inst) { // The URL reaches the script through the environment so the shell never // parses it as code. setupCmd += ` --database-url "${DATABASE_URL}"` @@ -588,6 +634,14 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp `echo "Applying operator config..."`, `cp /config/warpgate.yaml /data/warpgate.yaml`, ) + if externalDatabase(inst) { + // Write database_url from the environment as a single-quoted YAML + // scalar ('' is the only escape needed inside one). + scriptParts = append(scriptParts, + `sed -i '/^database_url:/d' /data/warpgate.yaml`, + `printf "database_url: '%s'\n" "$(printf '%s' "${DATABASE_URL}" | sed "s/'/''/g")" >> /data/warpgate.yaml`, + ) + } } // 5. Generate self-signed TLS if no certs exist yet (and no TLS secret / cert-manager) @@ -625,8 +679,8 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp }, }, } - if inst.Spec.DatabaseURL != "" { - initEnv = append(initEnv, corev1.EnvVar{Name: "DATABASE_URL", Value: inst.Spec.DatabaseURL}) + if externalDatabase(inst) { + initEnv = append(initEnv, databaseURLEnv(inst)) } // Init container volume mounts @@ -681,7 +735,9 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp } } - hash := configHash(inst) + // Fold the init script into the rollout hash so pods pick up changes to + // the generated script (not just to the CR spec). + hash := fmt.Sprintf("%x", sha256.Sum256([]byte(configHash(inst)+initScript)))[:12] // Deployment strategy — default Recreate for RWO PVC compatibility. strategy := appsv1.DeploymentStrategy{ diff --git a/internal/controller/warpgateinstance_controller_test.go b/internal/controller/warpgateinstance_controller_test.go index e4dcb2a..b2394d8 100644 --- a/internal/controller/warpgateinstance_controller_test.go +++ b/internal/controller/warpgateinstance_controller_test.go @@ -28,6 +28,7 @@ import ( "k8s.io/client-go/kubernetes/scheme" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" + sigsyaml "sigs.k8s.io/yaml" warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" ) @@ -2261,7 +2262,7 @@ var _ = Describe("WarpgateInstance Controller", func() { } }) - It("should generate config with postgres database_url instead of sqlite", func() { + It("should keep the database URL out of the ConfigMap and render it in the init container", func() { nn := types.NamespacedName{Name: instName, Namespace: testNamespace} _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) @@ -2273,8 +2274,15 @@ var _ = Describe("WarpgateInstance Controller", func() { }, &cm)).To(Succeed()) yaml := cm.Data["warpgate.yaml"] - Expect(yaml).To(ContainSubstring("database_url: \"postgres://host/db\"")) + Expect(yaml).NotTo(ContainSubstring("database_url")) + Expect(yaml).NotTo(ContainSubstring("postgres://host/db")) Expect(yaml).NotTo(ContainSubstring("sqlite:")) + + var deploy appsv1.Deployment + Expect(k8sClient.Get(ctx, nn, &deploy)).To(Succeed()) + script := deploy.Spec.Template.Spec.InitContainers[0].Command[2] + Expect(script).To(ContainSubstring(`printf "database_url: '%s'\n"`)) + Expect(script).To(ContainSubstring(`"${DATABASE_URL}"`)) }) }) @@ -3126,6 +3134,18 @@ var _ = Describe("WarpgateInstance Controller", func() { Expect(resolveImage(inst)).To(Equal("ghcr.io/warp-tech/warpgate:v0.21.1")) }) + It("configHash changes when databaseURLSecretRef changes", func() { + base := &warpgatev1alpha1.WarpgateInstance{ + Spec: warpgatev1alpha1.WarpgateInstanceSpec{Version: "0.21.1"}, + } + withRef := base.DeepCopy() + withRef.Spec.DatabaseURLSecretRef = &warpgatev1alpha1.SecretKeyRef{Name: "db"} + otherKey := withRef.DeepCopy() + otherKey.Spec.DatabaseURLSecretRef.Key = "uri" + Expect(configHash(base)).NotTo(Equal(configHash(withRef))) + Expect(configHash(withRef)).NotTo(Equal(configHash(otherKey))) + }) + It("configHash changes when DatabaseURL changes", func() { base := &warpgatev1alpha1.WarpgateInstance{ Spec: warpgatev1alpha1.WarpgateInstanceSpec{Version: "0.21.1"}, @@ -3157,7 +3177,7 @@ var _ = Describe("WarpgateInstance Controller", func() { // 22. buildWarpgateConfig edge cases // ----------------------------------------------------------------------- Context("buildWarpgateConfig edge cases", func() { - It("should include database_url postgres section when DatabaseURL is set", func() { + It("should leave database_url out of the generated config when DatabaseURL is set", func() { inst := &warpgatev1alpha1.WarpgateInstance{ ObjectMeta: metav1.ObjectMeta{ Name: "cfg-dburl", @@ -3173,10 +3193,45 @@ var _ = Describe("WarpgateInstance Controller", func() { }, } yaml := reconciler.buildWarpgateConfig(inst) - Expect(yaml).To(ContainSubstring("database_url: \"postgres://user:pass@host:5432/warpgate\"")) + Expect(yaml).NotTo(ContainSubstring("database_url")) + Expect(yaml).NotTo(ContainSubstring("user:pass")) Expect(yaml).NotTo(ContainSubstring("sqlite")) }) + It("should leave database_url out of the generated config when databaseURLSecretRef is set", func() { + inst := &warpgatev1alpha1.WarpgateInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cfg-dburl-ref", + Namespace: testNamespace, + }, + Spec: warpgatev1alpha1.WarpgateInstanceSpec{ + Version: "0.21.1", + DatabaseURLSecretRef: &warpgatev1alpha1.SecretKeyRef{Name: "db"}, + }, + } + yaml := reconciler.buildWarpgateConfig(inst) + Expect(yaml).NotTo(ContainSubstring("database_url")) + }) + + It("should YAML-escape externalHost instead of interpolating it raw", func() { + inst := &warpgatev1alpha1.WarpgateInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "cfg-exthost-escape", + Namespace: testNamespace, + }, + Spec: warpgatev1alpha1.WarpgateInstanceSpec{ + Version: "0.21.1", + ExternalHost: "evil.example.com\ndatabase_url: postgres://attacker/db", + }, + } + out := reconciler.buildWarpgateConfig(inst) + + var parsed map[string]any + Expect(sigsyaml.Unmarshal([]byte(out), &parsed)).To(Succeed()) + Expect(parsed["external_host"]).To(Equal(inst.Spec.ExternalHost)) + Expect(parsed["database_url"]).To(Equal("sqlite:/data/db")) + }) + It("should use sqlite when DatabaseURL is empty", func() { inst := &warpgatev1alpha1.WarpgateInstance{ ObjectMeta: metav1.ObjectMeta{ @@ -3524,6 +3579,50 @@ var _ = Describe("WarpgateInstance Controller", func() { Expect(initCtr.Env).To(ContainElement(corev1.EnvVar{Name: "DATABASE_URL", Value: hostile})) }) + It("should source DATABASE_URL from databaseURLSecretRef and prefer it over databaseURL", func() { + inst := &warpgatev1alpha1.WarpgateInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: "deploy-dburl-ref", + Namespace: testNamespace, + }, + Spec: warpgatev1alpha1.WarpgateInstanceSpec{ + Version: "0.21.1", + AdminPasswordSecretRef: warpgatev1alpha1.SecretKeyRef{ + Name: "dummy", + }, + TLS: &warpgatev1alpha1.InstanceTLSSpec{ + CertManager: boolPtr(false), + }, + DatabaseURL: "postgres://inline/ignored", + DatabaseURLSecretRef: &warpgatev1alpha1.SecretKeyRef{Name: "wg-db"}, + }, + } + deploy := reconciler.buildDeployment(inst) + initCtr := deploy.Spec.Template.Spec.InitContainers[0] + Expect(initCtr.Command[2]).To(ContainSubstring(`--database-url "${DATABASE_URL}"`)) + Expect(initCtr.Command[2]).NotTo(ContainSubstring("inline")) + + var dbEnv *corev1.EnvVar + for i := range initCtr.Env { + if initCtr.Env[i].Name == "DATABASE_URL" { + dbEnv = &initCtr.Env[i] + } + } + Expect(dbEnv).NotTo(BeNil()) + Expect(dbEnv.Value).To(BeEmpty()) + Expect(dbEnv.ValueFrom).NotTo(BeNil()) + Expect(dbEnv.ValueFrom.SecretKeyRef.Name).To(Equal("wg-db")) + Expect(dbEnv.ValueFrom.SecretKeyRef.Key).To(Equal("url")) + + inst.Spec.DatabaseURLSecretRef.Key = "uri" + deploy = reconciler.buildDeployment(inst) + for _, e := range deploy.Spec.Template.Spec.InitContainers[0].Env { + if e.Name == "DATABASE_URL" { + Expect(e.ValueFrom.SecretKeyRef.Key).To(Equal("uri")) + } + } + }) + It("should not set DATABASE_URL when DatabaseURL is empty", func() { inst := &warpgatev1alpha1.WarpgateInstance{ ObjectMeta: metav1.ObjectMeta{ diff --git a/openspec/specs/instance-management/spec.md b/openspec/specs/instance-management/spec.md index 6288505..ebe17fb 100644 --- a/openspec/specs/instance-management/spec.md +++ b/openspec/specs/instance-management/spec.md @@ -27,6 +27,8 @@ The operator provides a `WarpgateInstance` custom resource with the following sp - `tolerations` (optional) -- scheduling tolerations. - `createConnection` (optional, default `true`) -- auto-create a `WarpgateConnection` CR. - `externalHost` (optional) -- external hostname for cookie domain and URL generation. +- `databaseURLSecretRef` (optional) -- Secret key (`name`, `key` defaulting to `url`) holding an external database URL. Takes precedence over `databaseURL`. +- `databaseURL` (optional, deprecated) -- inline external database URL, stored in plain text. Status fields: @@ -113,3 +115,16 @@ The init container runs a generated `sh -c` script. User-controlled values (admi **Scenarios:** - **Given** a `WarpgateInstance` whose `databaseURL` contains shell metacharacters (`"`, `;`, `$(...)`, backticks) **When** the controller builds the Deployment **Then** the init script only references `${DATABASE_URL}` and the raw value appears solely in the init container's `DATABASE_URL` environment variable. + +### REQ-INST-008: Database Credentials Stay Out of the ConfigMap + +**Status:** ADDED + +When an external database is configured (`databaseURLSecretRef` or the deprecated `databaseURL`), the generated `warpgate.yaml` ConfigMap omits `database_url`. The init container gets the URL as `DATABASE_URL` (from the referenced Secret when set) and appends it to `/data/warpgate.yaml` as a single-quoted YAML scalar. Other interpolated values such as `externalHost` are emitted through a YAML marshaller. The validating webhook warns when `databaseURL` is used, and when it is ignored because `databaseURLSecretRef` is also set. + +**Scenarios:** + +- **Given** a `WarpgateInstance` with `databaseURLSecretRef` **When** the controller reconciles **Then** the ConfigMap contains no `database_url` and the init container's `DATABASE_URL` uses a `secretKeyRef`. +- **Given** a `WarpgateInstance` with only `databaseURL` **When** it is admitted **Then** the webhook returns a deprecation warning and the instance keeps working. +- **Given** an `externalHost` containing a newline and extra YAML keys **When** the config is generated **Then** it parses back to the exact `externalHost` string and no extra keys appear. +- **Given** the operator changes the generated init script **When** it reconciles an existing instance **Then** the pod template hash changes and the Deployment rolls out. From 8f3ef969cc5d5c66e9fd2eb88130fcf47fe80bf6 Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:44:11 +0300 Subject: [PATCH 3/9] fix(target)!: verify target TLS certificates unless told otherwise TLSConfigSpec.verify was a plain bool with omitempty, so a tls block that only set a mode (e.g. mode: Required) was sent to Warpgate with verification off, and certificate checks were silently skipped. verify is now a *bool with a CRD default of true, the webhook defaults it as well, and toWarpgateTLS treats nil as true. Verification is only turned off when the spec sets verify to false explicitly. BREAKING CHANGE: WarpgateTarget tls blocks without a verify field now verify the target certificate. Targets with self-signed or otherwise untrusted certificates that relied on the old implicit false need an explicit verify: false. --- api/v1alpha1/warpgatetarget_types.go | 7 ++- api/v1alpha1/warpgatetarget_webhook.go | 33 ++++++++---- api/v1alpha1/warpgatetarget_webhook_test.go | 36 +++++++++++++ api/v1alpha1/zz_generated.deepcopy.go | 13 +++-- .../templates/crds/warpgatetargets.yaml | 20 ++++++-- ...te.warpgate.warp.tech_warpgatetargets.yaml | 20 ++++++-- docs/crds/warpgate-target.md | 4 +- .../controller/warpgatetarget_controller.go | 4 +- .../warpgatetarget_controller_test.go | 51 +++++++++++++++++-- openspec/specs/target-management/spec.md | 1 + 10 files changed, 161 insertions(+), 28 deletions(-) diff --git a/api/v1alpha1/warpgatetarget_types.go b/api/v1alpha1/warpgatetarget_types.go index b813153..2d14c89 100644 --- a/api/v1alpha1/warpgatetarget_types.go +++ b/api/v1alpha1/warpgatetarget_types.go @@ -25,8 +25,11 @@ type TLSConfigSpec struct { // mode is the TLS mode: Disabled, Preferred, or Required. // +kubebuilder:validation:Enum=Disabled;Preferred;Required Mode string `json:"mode"` - // verify enables TLS certificate verification. - Verify bool `json:"verify,omitempty"` + // verify enables TLS certificate verification. Defaults to true; set it to + // false explicitly to accept unverified certificates. + // +optional + // +kubebuilder:default=true + Verify *bool `json:"verify,omitempty"` } // SSHTargetSpec defines the configuration for an SSH target. diff --git a/api/v1alpha1/warpgatetarget_webhook.go b/api/v1alpha1/warpgatetarget_webhook.go index 80ecf6f..3f4ad2f 100644 --- a/api/v1alpha1/warpgatetarget_webhook.go +++ b/api/v1alpha1/warpgatetarget_webhook.go @@ -86,23 +86,38 @@ func (d *WarpgateTargetCustomDefaulter) Default(ctx context.Context, target *War target.Spec.VNC.Port = 5900 } - // Default TLS mode to defaultTLSMode for HTTP, MySQL, and PostgreSQL targets. - if target.Spec.HTTP != nil && target.Spec.HTTP.TLS != nil && target.Spec.HTTP.TLS.Mode == "" { - target.Spec.HTTP.TLS.Mode = defaultTLSMode + // Default TLS mode and verification for HTTP, MySQL, PostgreSQL, and Kubernetes targets. + if target.Spec.HTTP != nil { + defaultTLS(target.Spec.HTTP.TLS) } - if target.Spec.MySQL != nil && target.Spec.MySQL.TLS != nil && target.Spec.MySQL.TLS.Mode == "" { - target.Spec.MySQL.TLS.Mode = defaultTLSMode + if target.Spec.MySQL != nil { + defaultTLS(target.Spec.MySQL.TLS) } - if target.Spec.PostgreSQL != nil && target.Spec.PostgreSQL.TLS != nil && target.Spec.PostgreSQL.TLS.Mode == "" { - target.Spec.PostgreSQL.TLS.Mode = defaultTLSMode + if target.Spec.PostgreSQL != nil { + defaultTLS(target.Spec.PostgreSQL.TLS) } - if target.Spec.Kubernetes != nil && target.Spec.Kubernetes.TLS != nil && target.Spec.Kubernetes.TLS.Mode == "" { - target.Spec.Kubernetes.TLS.Mode = defaultTLSMode + if target.Spec.Kubernetes != nil { + defaultTLS(target.Spec.Kubernetes.TLS) } return nil } +// defaultTLS fills in an explicit tls block: mode defaults to Preferred and +// certificate verification defaults to on. +func defaultTLS(tls *TLSConfigSpec) { + if tls == nil { + return + } + if tls.Mode == "" { + tls.Mode = defaultTLSMode + } + if tls.Verify == nil { + v := true + tls.Verify = &v + } +} + // +kubebuilder:webhook:path=/validate-warpgate-warpgate-warp-tech-v1alpha1-warpgatetarget,mutating=false,failurePolicy=fail,sideEffects=None,groups=warpgate.warpgate.warp.tech,resources=warpgatetargets,verbs=create;update;delete,versions=v1alpha1,name=vwarpgatetarget.kb.io,admissionReviewVersions=v1 // ValidateCreate implements admission.Validator. diff --git a/api/v1alpha1/warpgatetarget_webhook_test.go b/api/v1alpha1/warpgatetarget_webhook_test.go index 3e38e04..898bfed 100644 --- a/api/v1alpha1/warpgatetarget_webhook_test.go +++ b/api/v1alpha1/warpgatetarget_webhook_test.go @@ -501,3 +501,39 @@ func TestValidate_DeleteAlwaysSucceeds(t *testing.T) { t.Errorf("delete validation should always succeed: %v", err) } } + +func TestDefault_TLSVerifyDefaultsToTrue(t *testing.T) { + target := validHTTPTarget() + target.Spec.HTTP.TLS = &TLSConfigSpec{Mode: "Required"} + + if err := (&WarpgateTargetCustomDefaulter{}).Default(context.Background(), target); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Spec.HTTP.TLS.Verify == nil || !*target.Spec.HTTP.TLS.Verify { + t.Errorf("expected verify to default to true, got %v", target.Spec.HTTP.TLS.Verify) + } +} + +func TestDefault_TLSVerifyFalsePreserved(t *testing.T) { + target := validMySQLTarget() + target.Spec.MySQL.TLS = &TLSConfigSpec{Mode: "Preferred", Verify: boolPtr(false)} + + if err := (&WarpgateTargetCustomDefaulter{}).Default(context.Background(), target); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Spec.MySQL.TLS.Verify == nil || *target.Spec.MySQL.TLS.Verify { + t.Errorf("expected explicit verify=false to be kept, got %v", target.Spec.MySQL.TLS.Verify) + } +} + +func TestDefault_TLSAbsentStaysAbsent(t *testing.T) { + target := validPostgreSQLTarget() + target.Spec.PostgreSQL.TLS = nil + + if err := (&WarpgateTargetCustomDefaulter{}).Default(context.Background(), target); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if target.Spec.PostgreSQL.TLS != nil { + t.Errorf("expected no tls block to be added, got %+v", target.Spec.PostgreSQL.TLS) + } +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index dc86128..6eedb4e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -137,7 +137,7 @@ func (in *HTTPTargetSpec) DeepCopyInto(out *HTTPTargetSpec) { if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSConfigSpec) - **out = **in + (*in).DeepCopyInto(*out) } if in.Headers != nil { in, out := &in.Headers, &out.Headers @@ -295,7 +295,7 @@ func (in *KubernetesTargetSpec) DeepCopyInto(out *KubernetesTargetSpec) { if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSConfigSpec) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -320,7 +320,7 @@ func (in *MySQLTargetSpec) DeepCopyInto(out *MySQLTargetSpec) { if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSConfigSpec) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -345,7 +345,7 @@ func (in *PostgreSQLTargetSpec) DeepCopyInto(out *PostgreSQLTargetSpec) { if in.TLS != nil { in, out := &in.TLS, &out.TLS *out = new(TLSConfigSpec) - **out = **in + (*in).DeepCopyInto(*out) } } @@ -492,6 +492,11 @@ func (in *StorageSpec) DeepCopy() *StorageSpec { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *TLSConfigSpec) DeepCopyInto(out *TLSConfigSpec) { *out = *in + if in.Verify != nil { + in, out := &in.Verify, &out.Verify + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TLSConfigSpec. diff --git a/charts/warpgate-operator/templates/crds/warpgatetargets.yaml b/charts/warpgate-operator/templates/crds/warpgatetargets.yaml index 7fc49c6..2195297 100644 --- a/charts/warpgate-operator/templates/crds/warpgatetargets.yaml +++ b/charts/warpgate-operator/templates/crds/warpgatetargets.yaml @@ -87,7 +87,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode @@ -154,7 +157,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode @@ -226,7 +232,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode @@ -299,7 +308,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode diff --git a/config/crd/bases/warpgate.warpgate.warp.tech_warpgatetargets.yaml b/config/crd/bases/warpgate.warpgate.warp.tech_warpgatetargets.yaml index 42f7db7..7ec9b50 100644 --- a/config/crd/bases/warpgate.warpgate.warp.tech_warpgatetargets.yaml +++ b/config/crd/bases/warpgate.warpgate.warp.tech_warpgatetargets.yaml @@ -86,7 +86,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode @@ -153,7 +156,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode @@ -225,7 +231,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode @@ -298,7 +307,10 @@ spec: - Required type: string verify: - description: verify enables TLS certificate verification. + default: true + description: |- + verify enables TLS certificate verification. Defaults to true; set it to + false explicitly to accept unverified certificates. type: boolean required: - mode diff --git a/docs/crds/warpgate-target.md b/docs/crds/warpgate-target.md index 5ab9a1c..75dd216 100644 --- a/docs/crds/warpgate-target.md +++ b/docs/crds/warpgate-target.md @@ -111,9 +111,11 @@ Seven target types are supported: SSH, HTTP, MySQL, PostgreSQL, Kubernetes, RDP, | Field | Type | Required | Default | Description | |-------|------|----------|---------|-------------| | `tls.mode` | `string` | Yes | - | TLS mode: `Disabled`, `Preferred`, or `Required` | -| `tls.verify` | `bool` | No | `false` | Enable TLS certificate verification | +| `tls.verify` | `bool` | No | `true` | Verify the target's TLS certificate. Set `false` explicitly to accept unverified certificates | When the `tls` block is omitted entirely, Warpgate's own default applies: mode `Preferred` with verification on. +A `tls` block that leaves out `verify` also verifies. Before this default existed, a block such as +`tls: {mode: Required}` silently turned verification off; add `verify: false` if you really need that. ### SecretKeyRef (shared) diff --git a/internal/controller/warpgatetarget_controller.go b/internal/controller/warpgatetarget_controller.go index 508022d..cca9eb7 100644 --- a/internal/controller/warpgatetarget_controller.go +++ b/internal/controller/warpgatetarget_controller.go @@ -380,11 +380,13 @@ func (r *WarpgateTargetReconciler) databaseAuth(ctx context.Context, namespace, } // toWarpgateTLS converts an optional CRD TLS block; absent means Warpgate's default (Preferred, verify). +// Verification is only turned off when the spec says verify: false explicitly. func toWarpgateTLS(spec *warpgatev1alpha1.TLSConfigSpec) warpgate.TLSConfig { if spec == nil { return warpgate.TLSConfig{Mode: "Preferred", Verify: true} } - return warpgate.TLSConfig{Mode: spec.Mode, Verify: spec.Verify} + verify := spec.Verify == nil || *spec.Verify + return warpgate.TLSConfig{Mode: spec.Mode, Verify: verify} } func boolValue(b *bool) bool { diff --git a/internal/controller/warpgatetarget_controller_test.go b/internal/controller/warpgatetarget_controller_test.go index 5ad322c..5be8333 100644 --- a/internal/controller/warpgatetarget_controller_test.go +++ b/internal/controller/warpgatetarget_controller_test.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/reconcile" warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" + "github.com/thereisnotime/warpgate-operator/internal/warpgate" ) var _ = Describe("WarpgateTarget Controller", func() { @@ -44,6 +45,50 @@ var _ = Describe("WarpgateTarget Controller", func() { const targetRequeueAfter = 5 * time.Minute + verifyPtr := func(b bool) *bool { return &b } + + Context("toWarpgateTLS", func() { + It("verifies when the tls block is absent", func() { + Expect(toWarpgateTLS(nil)).To(Equal(warpgate.TLSConfig{Mode: "Preferred", Verify: true})) + }) + + It("verifies when verify is not set", func() { + got := toWarpgateTLS(&warpgatev1alpha1.TLSConfigSpec{Mode: "Required"}) + Expect(got).To(Equal(warpgate.TLSConfig{Mode: "Required", Verify: true})) + }) + + It("verifies when verify is true", func() { + got := toWarpgateTLS(&warpgatev1alpha1.TLSConfigSpec{Mode: "Required", Verify: verifyPtr(true)}) + Expect(got.Verify).To(BeTrue()) + }) + + It("skips verification only when verify is explicitly false", func() { + got := toWarpgateTLS(&warpgatev1alpha1.TLSConfigSpec{Mode: "Preferred", Verify: verifyPtr(false)}) + Expect(got).To(Equal(warpgate.TLSConfig{Mode: "Preferred", Verify: false})) + }) + + It("gets verify=true from the CRD default when a tls block omits it", func() { + target := &warpgatev1alpha1.WarpgateTarget{ + ObjectMeta: metav1.ObjectMeta{Name: "tls-default-verify", Namespace: testNamespace}, + Spec: warpgatev1alpha1.WarpgateTargetSpec{ + ConnectionRef: "unused", + Name: "tls-default-verify", + HTTP: &warpgatev1alpha1.HTTPTargetSpec{ + URL: "https://app.example.com", + TLS: &warpgatev1alpha1.TLSConfigSpec{Mode: "Required"}, + }, + }, + } + Expect(k8sClient.Create(ctx, target)).To(Succeed()) + DeferCleanup(func() { _ = k8sClient.Delete(ctx, target) }) + + var stored warpgatev1alpha1.WarpgateTarget + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: target.Name, Namespace: testNamespace}, &stored)).To(Succeed()) + Expect(stored.Spec.HTTP.TLS.Verify).NotTo(BeNil()) + Expect(*stored.Spec.HTTP.TLS.Verify).To(BeTrue()) + }) + }) + BeforeEach(func() { reconciler = &WarpgateTargetReconciler{ Client: k8sClient, @@ -645,7 +690,7 @@ var _ = Describe("WarpgateTarget Controller", func() { }, TLS: &warpgatev1alpha1.TLSConfigSpec{ Mode: "Required", - Verify: true, + Verify: verifyPtr(true), }, }, }, @@ -767,7 +812,7 @@ var _ = Describe("WarpgateTarget Controller", func() { }, TLS: &warpgatev1alpha1.TLSConfigSpec{ Mode: "Preferred", - Verify: false, + Verify: verifyPtr(false), }, }, }, @@ -1466,7 +1511,7 @@ var _ = Describe("WarpgateTarget Controller", func() { ExternalHost: "app.example.com", TLS: &warpgatev1alpha1.TLSConfigSpec{ Mode: "Required", - Verify: true, + Verify: verifyPtr(true), }, }, }, diff --git a/openspec/specs/target-management/spec.md b/openspec/specs/target-management/spec.md index 62b2a82..8bb7b89 100644 --- a/openspec/specs/target-management/spec.md +++ b/openspec/specs/target-management/spec.md @@ -46,6 +46,7 @@ HTTP targets support `url`, optional `tls` configuration (mode: `Disabled`/`Pref **Scenarios:** - **Given** an HTTP target with TLS mode `Required` and verify `true` **When** the controller reconciles **Then** the Warpgate API request includes the TLS configuration. +- **Given** a target whose `tls` block omits `verify` **When** it is admitted and reconciled **Then** `verify` defaults to `true` (CRD default and webhook) and the Warpgate API request has `verify: true`. Verification is only disabled when `verify: false` is set explicitly. ### REQ-TARGET-004: MySQL Target **Status:** ADDED From 569ba2f32e87af1f2c2781b39e06bf4ec5c5de1f Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:48:57 +0300 Subject: [PATCH 4/9] fix(credential): propagate password Secret rotation to Warpgate The password credential controller only read its Secret on reconcile and never watched it, and once a credential existed it was never touched again. Rotating the Secret left the old password valid in Warpgate. Watch Secrets and enqueue the credentials in the same namespace that reference them. Track the applied Secret revision in status.appliedSecretVersion; when it changes (or the user changed), delete the old Warpgate credential and create a new one with the current password. Warpgate has no in-place password update, so the credential ID changes on rotation. --- .../warpgatepasswordcredential_types.go | 6 + .../crds/warpgatepasswordcredentials.yaml | 6 + ...warp.tech_warpgatepasswordcredentials.yaml | 6 + docs/crds/warpgate-password-credential.md | 5 + .../warpgatepasswordcredential_controller.go | 60 +++- ...arpgatepasswordcredential_rotation_test.go | 316 ++++++++++++++++++ openspec/specs/credential-management/spec.md | 13 + 7 files changed, 411 insertions(+), 1 deletion(-) create mode 100644 internal/controller/warpgatepasswordcredential_rotation_test.go diff --git a/api/v1alpha1/warpgatepasswordcredential_types.go b/api/v1alpha1/warpgatepasswordcredential_types.go index 41e5329..98e4fb7 100644 --- a/api/v1alpha1/warpgatepasswordcredential_types.go +++ b/api/v1alpha1/warpgatepasswordcredential_types.go @@ -39,6 +39,12 @@ type WarpgatePasswordCredentialStatus struct { UserID string `json:"userID,omitempty"` // credentialID is the Warpgate-assigned credential UUID. CredentialID string `json:"credentialID,omitempty"` + + // appliedSecretVersion identifies the password Secret revision + // (/@) last pushed to Warpgate. When the Secret + // changes, the credential is replaced so the old password stops working. + // +optional + AppliedSecretVersion string `json:"appliedSecretVersion,omitempty"` // conditions represent the current state. // +listType=map // +listMapKey=type diff --git a/charts/warpgate-operator/templates/crds/warpgatepasswordcredentials.yaml b/charts/warpgate-operator/templates/crds/warpgatepasswordcredentials.yaml index 0fea996..69e7a30 100644 --- a/charts/warpgate-operator/templates/crds/warpgatepasswordcredentials.yaml +++ b/charts/warpgate-operator/templates/crds/warpgatepasswordcredentials.yaml @@ -82,6 +82,12 @@ spec: status: description: status defines the observed state of WarpgatePasswordCredential. properties: + appliedSecretVersion: + description: |- + appliedSecretVersion identifies the password Secret revision + (/@) last pushed to Warpgate. When the Secret + changes, the credential is replaced so the old password stops working. + type: string conditions: description: conditions represent the current state. items: diff --git a/config/crd/bases/warpgate.warpgate.warp.tech_warpgatepasswordcredentials.yaml b/config/crd/bases/warpgate.warpgate.warp.tech_warpgatepasswordcredentials.yaml index 1dea8db..77a4f0b 100644 --- a/config/crd/bases/warpgate.warpgate.warp.tech_warpgatepasswordcredentials.yaml +++ b/config/crd/bases/warpgate.warpgate.warp.tech_warpgatepasswordcredentials.yaml @@ -81,6 +81,12 @@ spec: status: description: status defines the observed state of WarpgatePasswordCredential. properties: + appliedSecretVersion: + description: |- + appliedSecretVersion identifies the password Secret revision + (/@) last pushed to Warpgate. When the Secret + changes, the credential is replaced so the old password stops working. + type: string conditions: description: conditions represent the current state. items: diff --git a/docs/crds/warpgate-password-credential.md b/docs/crds/warpgate-password-credential.md index 4799c15..c7efbc5 100644 --- a/docs/crds/warpgate-password-credential.md +++ b/docs/crds/warpgate-password-credential.md @@ -19,6 +19,7 @@ This is useful when you want explicit control over password credentials. For aut |-------|------|-------------| | `userID` | `string` | Resolved Warpgate user UUID | | `credentialID` | `string` | Warpgate-assigned credential UUID | +| `appliedSecretVersion` | `string` | `/@` of the password Secret last pushed to Warpgate | | `conditions` | `[]Condition` | Standard Kubernetes conditions | ## Print Columns @@ -72,4 +73,8 @@ The following defaults are applied on create and update: - The referenced Secret must exist in the same namespace as the CR. - The user specified by `username` must already exist in Warpgate. - Deleting the CR removes the password credential from the Warpgate user via the finalizer. +- The operator watches the referenced Secret. When it changes, the old Warpgate credential is deleted and a new one + is created with the new password, so the previous password stops working right away. Warpgate has no in-place + password update, which is why the credential ID changes on rotation. Credentials created by older operator versions + are re-applied once after upgrading, since there is no recorded Secret version for them yet. - If you just need a basic auto-generated password, use `WarpgateUser` with `generatePassword: true` instead of creating this resource manually. diff --git a/internal/controller/warpgatepasswordcredential_controller.go b/internal/controller/warpgatepasswordcredential_controller.go index 46b0972..af25bea 100644 --- a/internal/controller/warpgatepasswordcredential_controller.go +++ b/internal/controller/warpgatepasswordcredential_controller.go @@ -29,7 +29,9 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "sigs.k8s.io/controller-runtime/pkg/handler" logf "sigs.k8s.io/controller-runtime/pkg/log" + "sigs.k8s.io/controller-runtime/pkg/reconcile" warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" "github.com/thereisnotime/warpgate-operator/internal/warpgate" @@ -114,6 +116,7 @@ func (r *WarpgatePasswordCredentialReconciler) Reconcile(ctx context.Context, re _ = r.Status().Update(ctx, &cred) return ctrl.Result{}, err } + prevUserID := cred.Status.UserID cred.Status.UserID = wgUser.ID // Read the password from the referenced Secret. @@ -153,6 +156,34 @@ func (r *WarpgatePasswordCredentialReconciler) Reconcile(ctx context.Context, re return ctrl.Result{}, err } + // Warpgate can't update a password credential in place, so a rotated Secret + // (or a credential that now belongs to a different user) means replacing it. + // The old credential is removed first so the previous password never + // outlives the rotation. + version := passwordSecretVersion(&secret, key) + userChanged := prevUserID != "" && prevUserID != wgUser.ID + if cred.Status.CredentialID != "" && (cred.Status.AppliedSecretVersion != version || userChanged) { + owner := prevUserID + if owner == "" { + owner = wgUser.ID + } + if err := wgClient.DeletePasswordCredential(owner, cred.Status.CredentialID); err != nil && !warpgate.IsNotFound(err) { + log.Error(err, "Failed to delete outdated password credential", "credentialID", cred.Status.CredentialID) + meta.SetStatusCondition(&cred.Status.Conditions, metav1.Condition{ + Type: "Ready", + Status: metav1.ConditionFalse, + Reason: "RotateFailed", + Message: fmt.Sprintf("Failed to remove outdated password credential: %v", err), + ObservedGeneration: cred.Generation, + }) + _ = r.Status().Update(ctx, &cred) + return ctrl.Result{}, err + } + log.Info("Deleted outdated password credential", "credentialID", cred.Status.CredentialID) + cred.Status.CredentialID = "" + cred.Status.AppliedSecretVersion = "" + } + // Create the credential if it doesn't exist yet. if cred.Status.CredentialID == "" { created, err := wgClient.CreatePasswordCredential(wgUser.ID, string(password)) @@ -169,6 +200,7 @@ func (r *WarpgatePasswordCredentialReconciler) Reconcile(ctx context.Context, re return ctrl.Result{}, err } cred.Status.CredentialID = created.ID + cred.Status.AppliedSecretVersion = version log.Info("created password credential in Warpgate", "credentialID", created.ID) } @@ -187,10 +219,36 @@ func (r *WarpgatePasswordCredentialReconciler) Reconcile(ctx context.Context, re return ctrl.Result{RequeueAfter: r.ReconcileInterval}, nil } -// SetupWithManager sets up the controller with the Manager. +// passwordSecretVersion identifies the Secret revision a password was read from. +func passwordSecretVersion(secret *corev1.Secret, key string) string { + return fmt.Sprintf("%s/%s@%s", secret.Name, key, secret.ResourceVersion) +} + +// credentialsForSecret maps a Secret to the password credentials in its +// namespace that read their password from it. +func (r *WarpgatePasswordCredentialReconciler) credentialsForSecret(ctx context.Context, obj client.Object) []reconcile.Request { + var list warpgatev1alpha1.WarpgatePasswordCredentialList + if err := r.List(ctx, &list, client.InNamespace(obj.GetNamespace())); err != nil { + logf.FromContext(ctx).Error(err, "Failed to list password credentials for Secret", "secret", obj.GetName()) + return nil + } + var reqs []reconcile.Request + for _, cred := range list.Items { + if cred.Spec.PasswordSecretRef.Name == obj.GetName() { + reqs = append(reqs, reconcile.Request{NamespacedName: types.NamespacedName{ + Name: cred.Name, Namespace: cred.Namespace, + }}) + } + } + return reqs +} + +// SetupWithManager sets up the controller with the Manager. It also watches +// Secrets so a rotated password reaches Warpgate without waiting for a resync. func (r *WarpgatePasswordCredentialReconciler) SetupWithManager(mgr ctrl.Manager) error { return ctrl.NewControllerManagedBy(mgr). For(&warpgatev1alpha1.WarpgatePasswordCredential{}). + Watches(&corev1.Secret{}, handler.EnqueueRequestsFromMapFunc(r.credentialsForSecret)). Named("warpgatepasswordcredential"). Complete(r) } diff --git a/internal/controller/warpgatepasswordcredential_rotation_test.go b/internal/controller/warpgatepasswordcredential_rotation_test.go new file mode 100644 index 0000000..7ff4521 --- /dev/null +++ b/internal/controller/warpgatepasswordcredential_rotation_test.go @@ -0,0 +1,316 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/config" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" +) + +// rotationMock is a fake Warpgate that records password credential calls. +type rotationMock struct { + mu sync.Mutex + next int + created []string // passwords sent on create, in order + deleted []string // credential IDs deleted, in order + deleteErr bool +} + +func (m *rotationMock) snapshot() (created, deleted []string) { + m.mu.Lock() + defer m.mu.Unlock() + return append([]string(nil), m.created...), append([]string(nil), m.deleted...) +} + +func (m *rotationMock) server(userID, username string) *httptest.Server { + mux := http.NewServeMux() + mockLogin(mux) + mux.HandleFunc("/@warpgate/admin/api/users", func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode([]map[string]any{{"id": userID, "username": username}}) + }) + base := "/@warpgate/admin/api/users/" + userID + "/credentials/passwords" + mux.HandleFunc(base, func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + http.NotFound(w, r) + return + } + var body map[string]string + _ = json.NewDecoder(r.Body).Decode(&body) + m.mu.Lock() + m.next++ + id := fmt.Sprintf("cred-rot-%d", m.next) + m.created = append(m.created, body["password"]) + m.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusCreated) + _ = json.NewEncoder(w).Encode(map[string]any{"id": id, "password": "hashed"}) + }) + mux.HandleFunc(base+"/", func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + http.NotFound(w, r) + return + } + m.mu.Lock() + defer m.mu.Unlock() + if m.deleteErr { + http.Error(w, "boom", http.StatusInternalServerError) + return + } + m.deleted = append(m.deleted, strings.TrimPrefix(r.URL.Path, base+"/")) + w.WriteHeader(http.StatusNoContent) + }) + return httptest.NewServer(mux) +} + +var _ = Describe("WarpgatePasswordCredential Secret rotation", func() { + const ( + tokenSecret = "pwrot-token" + pwSecret = "pwrot-password" + connName = "pwrot-conn" + crName = "pwrot-cred" + otherCrName = "pwrot-other-cred" + ) + + var ( + mock *rotationMock + mockServer *httptest.Server + reconciler *WarpgatePasswordCredentialReconciler + nn types.NamespacedName + ) + + BeforeEach(func() { + mock = &rotationMock{} + mockServer = mock.server("user-rot", "rotuser") + reconciler = &WarpgatePasswordCredentialReconciler{Client: k8sClient, Scheme: k8sClient.Scheme()} + nn = types.NamespacedName{Name: crName, Namespace: testNamespace} + + Expect(k8sClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: tokenSecret, Namespace: testNamespace}, + Data: map[string][]byte{"token": []byte("t")}, + })).To(Succeed()) + Expect(k8sClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: pwSecret, Namespace: testNamespace}, + Data: map[string][]byte{"password": []byte("first-password")}, + })).To(Succeed()) + Expect(k8sClient.Create(ctx, &warpgatev1alpha1.WarpgateConnection{ + ObjectMeta: metav1.ObjectMeta{Name: connName, Namespace: testNamespace}, + Spec: warpgatev1alpha1.WarpgateConnectionSpec{ + Host: mockServer.URL, + AuthSecretRef: warpgatev1alpha1.AuthSecretRef{Name: tokenSecret}, + }, + })).To(Succeed()) + Expect(k8sClient.Create(ctx, &warpgatev1alpha1.WarpgatePasswordCredential{ + ObjectMeta: metav1.ObjectMeta{Name: crName, Namespace: testNamespace}, + Spec: warpgatev1alpha1.WarpgatePasswordCredentialSpec{ + ConnectionRef: connName, + Username: "rotuser", + PasswordSecretRef: warpgatev1alpha1.SecretKeyRef{Name: pwSecret}, + }, + })).To(Succeed()) + }) + + AfterEach(func() { + mockServer.Close() + for _, name := range []string{crName, otherCrName} { + cr := &warpgatev1alpha1.WarpgatePasswordCredential{} + if err := k8sClient.Get(ctx, types.NamespacedName{Name: name, Namespace: testNamespace}, cr); err == nil { + controllerutil.RemoveFinalizer(cr, passwordCredentialFinalizer) + _ = k8sClient.Update(ctx, cr) + _ = k8sClient.Delete(ctx, cr) + } + } + _ = k8sClient.Delete(ctx, &warpgatev1alpha1.WarpgateConnection{ + ObjectMeta: metav1.ObjectMeta{Name: connName, Namespace: testNamespace}, + }) + for _, name := range []string{tokenSecret, pwSecret} { + _ = k8sClient.Delete(ctx, &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: name, Namespace: testNamespace}}) + } + }) + + rotateSecret := func(password string) { + var s corev1.Secret + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: pwSecret, Namespace: testNamespace}, &s)).To(Succeed()) + s.Data["password"] = []byte(password) + Expect(k8sClient.Update(ctx, &s)).To(Succeed()) + } + + It("records the applied Secret version and does nothing while it is unchanged", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + + created, deleted := mock.snapshot() + Expect(created).To(Equal([]string{"first-password"})) + Expect(deleted).To(BeEmpty()) + + var cr warpgatev1alpha1.WarpgatePasswordCredential + Expect(k8sClient.Get(ctx, nn, &cr)).To(Succeed()) + Expect(cr.Status.CredentialID).To(Equal("cred-rot-1")) + Expect(cr.Status.AppliedSecretVersion).To(HavePrefix(pwSecret + "/password@")) + }) + + It("replaces the Warpgate credential when the Secret changes", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + + rotateSecret("second-password") + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + + created, deleted := mock.snapshot() + Expect(created).To(Equal([]string{"first-password", "second-password"})) + Expect(deleted).To(Equal([]string{"cred-rot-1"})) + + var cr warpgatev1alpha1.WarpgatePasswordCredential + Expect(k8sClient.Get(ctx, nn, &cr)).To(Succeed()) + Expect(cr.Status.CredentialID).To(Equal("cred-rot-2")) + readyCond := findReadyCondition(cr.Status.Conditions) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Status).To(Equal(metav1.ConditionTrue)) + }) + + It("re-applies once for credentials created before the version was tracked", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + + var cr warpgatev1alpha1.WarpgatePasswordCredential + Expect(k8sClient.Get(ctx, nn, &cr)).To(Succeed()) + cr.Status.AppliedSecretVersion = "" + Expect(k8sClient.Status().Update(ctx, &cr)).To(Succeed()) + + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + created, deleted := mock.snapshot() + Expect(created).To(HaveLen(2)) + Expect(deleted).To(Equal([]string{"cred-rot-1"})) + }) + + It("keeps the old credential ID and reports RotateFailed when the delete fails", func() { + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + + mock.mu.Lock() + mock.deleteErr = true + mock.mu.Unlock() + rotateSecret("second-password") + + _, err = reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).To(HaveOccurred()) + + created, _ := mock.snapshot() + Expect(created).To(Equal([]string{"first-password"})) + + var cr warpgatev1alpha1.WarpgatePasswordCredential + Expect(k8sClient.Get(ctx, nn, &cr)).To(Succeed()) + Expect(cr.Status.CredentialID).To(Equal("cred-rot-1")) + readyCond := findReadyCondition(cr.Status.Conditions) + Expect(readyCond).NotTo(BeNil()) + Expect(readyCond.Reason).To(Equal("RotateFailed")) + }) + + It("maps a Secret to the credentials that reference it", func() { + Expect(k8sClient.Create(ctx, &warpgatev1alpha1.WarpgatePasswordCredential{ + ObjectMeta: metav1.ObjectMeta{Name: otherCrName, Namespace: testNamespace}, + Spec: warpgatev1alpha1.WarpgatePasswordCredentialSpec{ + ConnectionRef: connName, + Username: "rotuser", + PasswordSecretRef: warpgatev1alpha1.SecretKeyRef{Name: "some-other-secret"}, + }, + })).To(Succeed()) + + reqs := reconciler.credentialsForSecret(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: pwSecret, Namespace: testNamespace}, + }) + Expect(reqs).To(Equal([]reconcile.Request{{NamespacedName: nn}})) + + Expect(reconciler.credentialsForSecret(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: pwSecret, Namespace: "kube-system"}, + })).To(BeEmpty()) + }) + + It("pushes a rotated Secret to Warpgate through the Secret watch", func() { + skip := true + mgr, err := ctrl.NewManager(cfg, ctrl.Options{ + Scheme: k8sClient.Scheme(), + Metrics: metricsserver.Options{BindAddress: "0"}, + Controller: config.Controller{SkipNameValidation: &skip}, + }) + Expect(err).NotTo(HaveOccurred()) + Expect((&WarpgatePasswordCredentialReconciler{ + Client: mgr.GetClient(), + Scheme: mgr.GetScheme(), + }).SetupWithManager(mgr)).To(Succeed()) + + mgrCtx, stop := context.WithCancel(ctx) + done := make(chan struct{}) + go func() { + defer GinkgoRecover() + defer close(done) + Expect(mgr.Start(mgrCtx)).To(Succeed()) + }() + DeferCleanup(func() { + stop() + <-done + }) + + // Wait for the initial create to land in status. + Eventually(func() string { + var cr warpgatev1alpha1.WarpgatePasswordCredential + if err := k8sClient.Get(ctx, nn, &cr); err != nil { + return "" + } + return cr.Status.AppliedSecretVersion + }, 20*time.Second, 200*time.Millisecond).ShouldNot(BeEmpty()) + created, _ := mock.snapshot() + Expect(created).To(ContainElement("first-password")) + Expect(created).NotTo(ContainElement("second-password")) + + // ReconcileInterval is zero, so nothing but the Secret watch can + // trigger the next reconcile. + rotateSecret("second-password") + + Eventually(func() []string { + created, _ := mock.snapshot() + return created + }, 20*time.Second, 200*time.Millisecond).Should(ContainElement("second-password")) + Eventually(func() []string { + _, deleted := mock.snapshot() + return deleted + }, 5*time.Second, 200*time.Millisecond).ShouldNot(BeEmpty()) + }) +}) diff --git a/openspec/specs/credential-management/spec.md b/openspec/specs/credential-management/spec.md index f4552c9..5950d8f 100644 --- a/openspec/specs/credential-management/spec.md +++ b/openspec/specs/credential-management/spec.md @@ -17,6 +17,7 @@ The operator provides a `WarpgatePasswordCredential` custom resource with the fo Status fields: - `userID` -- the resolved Warpgate user UUID. - `credentialID` -- the Warpgate-assigned credential UUID. +- `appliedSecretVersion` -- `/@` of the password Secret last pushed to Warpgate. - `conditions` -- standard Kubernetes conditions list. Print columns: `Username`, `CredentialID`, `Ready`. @@ -71,3 +72,15 @@ Both credential controllers resolve the target user by username via the Warpgate **Scenarios:** - **Given** a credential CR referencing username `"alice"` **When** the controller reconciles **Then** it calls the Warpgate API to resolve `"alice"` to a UUID and stores it in `status.userID`. + +### REQ-CRED-006: Password Rotation + +**Status:** ADDED + +The password credential controller watches Secrets and enqueues every `WarpgatePasswordCredential` in the same namespace whose `passwordSecretRef.name` matches. On reconcile it compares the Secret's `/@` with `status.appliedSecretVersion`. If they differ (or the credential now belongs to a different user), it deletes the old Warpgate credential first and then creates a new one with the current password, so the old password never outlives the rotation. + +**Scenarios:** + +- **Given** a synced `WarpgatePasswordCredential` **When** its password Secret is updated **Then** the Secret watch triggers a reconcile that deletes the old credential and creates one with the new password. +- **Given** the Secret is unchanged **When** the controller reconciles **Then** no Warpgate credential calls are made. +- **Given** deleting the old credential fails **When** the controller reconciles **Then** it keeps the old `credentialID`, sets `Ready=False` with reason `RotateFailed`, and retries. From e5fc04eccc96764db844e5aae4e2d232f7d356d5 Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:53:26 +0300 Subject: [PATCH 5/9] fix(instance)!: verify the auto-created connection when the CA is known The WarpgateConnection created for a WarpgateInstance always had insecureSkipVerify set, so the operator sent the admin password to whatever answered on the Service address, even when the instance served a user-provided certificate. WarpgateConnection gets an optional caSecretRef (PEM bundle, key defaults to ca.crt) that is used as the client's RootCAs. When an instance sets tls.secretName, its connection now verifies against that Secret's ca.crt, or tls.crt if there is no ca.crt. Verification is still skipped for the self-signed certificate the pod generates itself, since the operator never sees it. The new tls.verifyConnection field makes that explicit: true fails reconciliation instead of creating an unverified connection, false always skips verification. BREAKING CHANGE: instances with tls.secretName now get a verifying connection. The certificate must be valid for -http..svc; set tls.verifyConnection to false to keep the old behaviour if it isn't. --- api/v1alpha1/warpgateconnection_types.go | 6 + api/v1alpha1/warpgateconnection_webhook.go | 6 + .../warpgateconnection_webhook_test.go | 30 ++ api/v1alpha1/warpgateinstance_types.go | 9 + api/v1alpha1/warpgateinstance_webhook.go | 3 + api/v1alpha1/warpgateinstance_webhook_test.go | 19 ++ api/v1alpha1/zz_generated.deepcopy.go | 12 +- .../templates/crds/warpgateconnections.yaml | 17 ++ .../templates/crds/warpgateinstances.yaml | 9 + ...arpgate.warp.tech_warpgateconnections.yaml | 17 ++ ....warpgate.warp.tech_warpgateinstances.yaml | 9 + docs/crds/warpgate-connection.md | 3 + docs/crds/warpgate-instance.md | 10 + internal/controller/connection_tls_test.go | 258 ++++++++++++++++++ internal/controller/helpers.go | 30 ++ .../warpgateconnection_controller.go | 7 + .../controller/warpgateinstance_controller.go | 45 ++- internal/warpgate/client.go | 14 +- internal/warpgate/client_test.go | 84 ++++++ openspec/specs/connection-management/spec.md | 13 + openspec/specs/instance-management/spec.md | 15 +- 21 files changed, 612 insertions(+), 4 deletions(-) create mode 100644 internal/controller/connection_tls_test.go diff --git a/api/v1alpha1/warpgateconnection_types.go b/api/v1alpha1/warpgateconnection_types.go index fc1d47a..e8379d2 100644 --- a/api/v1alpha1/warpgateconnection_types.go +++ b/api/v1alpha1/warpgateconnection_types.go @@ -68,6 +68,12 @@ type WarpgateConnectionSpec struct { // insecureSkipVerify disables TLS certificate verification. Not recommended for production. // +optional InsecureSkipVerify bool `json:"insecureSkipVerify,omitempty"` + + // caSecretRef references a Secret key holding a PEM CA bundle used to verify the + // Warpgate server certificate instead of the system trust store. The key defaults + // to "ca.crt". Ignored when insecureSkipVerify is true. + // +optional + CASecretRef *SecretKeyRef `json:"caSecretRef,omitempty"` } // WarpgateConnectionStatus defines the observed state of WarpgateConnection. diff --git a/api/v1alpha1/warpgateconnection_webhook.go b/api/v1alpha1/warpgateconnection_webhook.go index 3d80441..b4c3d11 100644 --- a/api/v1alpha1/warpgateconnection_webhook.go +++ b/api/v1alpha1/warpgateconnection_webhook.go @@ -90,5 +90,11 @@ func validateConnection(conn *WarpgateConnection) (admission.Warnings, error) { if conn.Spec.AuthSecretRef.Name == "" { return nil, fmt.Errorf("spec.authSecretRef.name must not be empty") } + if conn.Spec.CASecretRef != nil && conn.Spec.CASecretRef.Name == "" { + return nil, fmt.Errorf("spec.caSecretRef.name must not be empty") + } + if conn.Spec.CASecretRef != nil && conn.Spec.InsecureSkipVerify { + return admission.Warnings{"caSecretRef is ignored because insecureSkipVerify is true"}, nil + } return nil, nil } diff --git a/api/v1alpha1/warpgateconnection_webhook_test.go b/api/v1alpha1/warpgateconnection_webhook_test.go index e93d9e3..4deefb8 100644 --- a/api/v1alpha1/warpgateconnection_webhook_test.go +++ b/api/v1alpha1/warpgateconnection_webhook_test.go @@ -123,6 +123,36 @@ func TestWarpgateConnectionValidator(t *testing.T) { } }) + t.Run("caSecretRef accepted", func(t *testing.T) { + c := validConn() + c.Spec.CASecretRef = &SecretKeyRef{Name: "warpgate-ca"} + warnings, err := v.ValidateCreate(ctx, c) + if err != nil || len(warnings) != 0 { + t.Errorf("expected no error or warnings, got %v / %v", err, warnings) + } + }) + + t.Run("caSecretRef without name rejected", func(t *testing.T) { + c := validConn() + c.Spec.CASecretRef = &SecretKeyRef{Key: "ca.crt"} + if _, err := v.ValidateCreate(ctx, c); err == nil { + t.Error("expected error for empty caSecretRef.name") + } + }) + + t.Run("caSecretRef with insecureSkipVerify warns", func(t *testing.T) { + c := validConn() + c.Spec.CASecretRef = &SecretKeyRef{Name: "warpgate-ca"} + c.Spec.InsecureSkipVerify = true + warnings, err := v.ValidateCreate(ctx, c) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(warnings) == 0 { + t.Error("expected a warning that caSecretRef is ignored") + } + }) + t.Run("update validation works", func(t *testing.T) { old := validConn() bad := validConn() diff --git a/api/v1alpha1/warpgateinstance_types.go b/api/v1alpha1/warpgateinstance_types.go index bbef5cb..f47d81f 100644 --- a/api/v1alpha1/warpgateinstance_types.go +++ b/api/v1alpha1/warpgateinstance_types.go @@ -203,6 +203,15 @@ type InstanceTLSSpec struct { // If set, cert-manager is not used and the secret is mounted directly. // +optional SecretName string `json:"secretName,omitempty"` + + // verifyConnection controls TLS verification on the auto-created WarpgateConnection. + // Unset: verify against the CA in secretName (its ca.crt, or tls.crt when there is no + // ca.crt) when secretName is set, and skip verification for the self-signed certificate + // generated inside the pod, which the operator has no copy of. + // true: always verify; reconciliation fails if the operator has no CA to verify against. + // false: never verify. + // +optional + VerifyConnection *bool `json:"verifyConnection,omitempty"` } // CertIssuerRef references a cert-manager Issuer or ClusterIssuer. diff --git a/api/v1alpha1/warpgateinstance_webhook.go b/api/v1alpha1/warpgateinstance_webhook.go index 31d7767..e177b34 100644 --- a/api/v1alpha1/warpgateinstance_webhook.go +++ b/api/v1alpha1/warpgateinstance_webhook.go @@ -275,6 +275,9 @@ func validateWarpgateInstance(inst *WarpgateInstance) (admission.Warnings, error } } + if tls := inst.Spec.TLS; tls != nil && tls.VerifyConnection != nil && *tls.VerifyConnection && tls.SecretName == "" { + warnings = append(warnings, "tls.verifyConnection is true but tls.secretName is not set; the operator has no CA to verify the auto-created connection against") + } dbErrs, dbWarnings := validateDatabase(inst, specPath) allErrs = append(allErrs, dbErrs...) warnings = append(warnings, dbWarnings...) diff --git a/api/v1alpha1/warpgateinstance_webhook_test.go b/api/v1alpha1/warpgateinstance_webhook_test.go index 21e5903..f929be3 100644 --- a/api/v1alpha1/warpgateinstance_webhook_test.go +++ b/api/v1alpha1/warpgateinstance_webhook_test.go @@ -693,3 +693,22 @@ func containsWarning(warnings []string, substr string) bool { } return false } + +func TestInstanceValidate_WarnVerifyConnectionWithoutSecret(t *testing.T) { + inst := validInstance() + inst.Spec.TLS = &InstanceTLSSpec{VerifyConnection: boolPtr(true)} + + warnings, err := (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !containsWarning(warnings, "verifyConnection") { + t.Errorf("expected a verifyConnection warning, got: %v", warnings) + } + + inst.Spec.TLS.SecretName = "warpgate-tls" + warnings, _ = (&WarpgateInstanceCustomValidator{}).ValidateCreate(context.Background(), inst) + if containsWarning(warnings, "verifyConnection") { + t.Errorf("did not expect a verifyConnection warning with secretName set, got: %v", warnings) + } +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 6eedb4e..84a20ae 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -262,6 +262,11 @@ func (in *InstanceTLSSpec) DeepCopyInto(out *InstanceTLSSpec) { *out = new(CertIssuerRef) **out = **in } + if in.VerifyConnection != nil { + in, out := &in.VerifyConnection, &out.VerifyConnection + *out = new(bool) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new InstanceTLSSpec. @@ -534,7 +539,7 @@ func (in *WarpgateConnection) DeepCopyInto(out *WarpgateConnection) { *out = *in out.TypeMeta = in.TypeMeta in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) - out.Spec = in.Spec + in.Spec.DeepCopyInto(&out.Spec) in.Status.DeepCopyInto(&out.Status) } @@ -622,6 +627,11 @@ func (in *WarpgateConnectionList) DeepCopyObject() runtime.Object { func (in *WarpgateConnectionSpec) DeepCopyInto(out *WarpgateConnectionSpec) { *out = *in out.AuthSecretRef = in.AuthSecretRef + if in.CASecretRef != nil { + in, out := &in.CASecretRef, &out.CASecretRef + *out = new(SecretKeyRef) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WarpgateConnectionSpec. diff --git a/charts/warpgate-operator/templates/crds/warpgateconnections.yaml b/charts/warpgate-operator/templates/crds/warpgateconnections.yaml index 27b8e0f..643d582 100644 --- a/charts/warpgate-operator/templates/crds/warpgateconnections.yaml +++ b/charts/warpgate-operator/templates/crds/warpgateconnections.yaml @@ -73,6 +73,23 @@ spec: required: - name type: object + caSecretRef: + description: |- + caSecretRef references a Secret key holding a PEM CA bundle used to verify the + Warpgate server certificate instead of the system trust store. The key defaults + to "ca.crt". Ignored when insecureSkipVerify is true. + properties: + key: + description: |- + key is the key within the Secret. Defaults to "password" for password refs. + Unused by tokenSecretRef (which always reads "username" and "password" keys). + type: string + name: + description: name is the name of the Secret. + type: string + required: + - name + type: object host: description: host is the URL of the Warpgate instance (e.g. https://warpgate.example.com). type: string diff --git a/charts/warpgate-operator/templates/crds/warpgateinstances.yaml b/charts/warpgate-operator/templates/crds/warpgateinstances.yaml index 7d0865a..7e2a1fd 100644 --- a/charts/warpgate-operator/templates/crds/warpgateinstances.yaml +++ b/charts/warpgate-operator/templates/crds/warpgateinstances.yaml @@ -372,6 +372,15 @@ spec: secretName references an existing TLS Secret (with tls.crt and tls.key). If set, cert-manager is not used and the secret is mounted directly. type: string + verifyConnection: + description: |- + verifyConnection controls TLS verification on the auto-created WarpgateConnection. + Unset: verify against the CA in secretName (its ca.crt, or tls.crt when there is no + ca.crt) when secretName is set, and skip verification for the self-signed certificate + generated inside the pod, which the operator has no copy of. + true: always verify; reconciliation fails if the operator has no CA to verify against. + false: never verify. + type: boolean type: object tolerations: description: tolerations for scheduling. diff --git a/config/crd/bases/warpgate.warpgate.warp.tech_warpgateconnections.yaml b/config/crd/bases/warpgate.warpgate.warp.tech_warpgateconnections.yaml index d069c38..b8900ce 100644 --- a/config/crd/bases/warpgate.warpgate.warp.tech_warpgateconnections.yaml +++ b/config/crd/bases/warpgate.warpgate.warp.tech_warpgateconnections.yaml @@ -72,6 +72,23 @@ spec: required: - name type: object + caSecretRef: + description: |- + caSecretRef references a Secret key holding a PEM CA bundle used to verify the + Warpgate server certificate instead of the system trust store. The key defaults + to "ca.crt". Ignored when insecureSkipVerify is true. + properties: + key: + description: |- + key is the key within the Secret. Defaults to "password" for password refs. + Unused by tokenSecretRef (which always reads "username" and "password" keys). + type: string + name: + description: name is the name of the Secret. + type: string + required: + - name + type: object host: description: host is the URL of the Warpgate instance (e.g. https://warpgate.example.com). type: string diff --git a/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml b/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml index 2e71b74..bcb0da0 100644 --- a/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml +++ b/config/crd/bases/warpgate.warpgate.warp.tech_warpgateinstances.yaml @@ -371,6 +371,15 @@ spec: secretName references an existing TLS Secret (with tls.crt and tls.key). If set, cert-manager is not used and the secret is mounted directly. type: string + verifyConnection: + description: |- + verifyConnection controls TLS verification on the auto-created WarpgateConnection. + Unset: verify against the CA in secretName (its ca.crt, or tls.crt when there is no + ca.crt) when secretName is set, and skip verification for the self-signed certificate + generated inside the pod, which the operator has no copy of. + true: always verify; reconciliation fails if the operator has no CA to verify against. + false: never verify. + type: boolean type: object tolerations: description: tolerations for scheduling. diff --git a/docs/crds/warpgate-connection.md b/docs/crds/warpgate-connection.md index f8731f2..2e8bf0b 100644 --- a/docs/crds/warpgate-connection.md +++ b/docs/crds/warpgate-connection.md @@ -21,6 +21,8 @@ Auth mode is auto-detected from the referenced Secret: if the `token` key exists | `authSecretRef.usernameKey` | `string` | No | `username` | Key in the Secret that holds the username (session auth) | | `authSecretRef.passwordKey` | `string` | No | `password` | Key in the Secret that holds the password (session auth) | | `insecureSkipVerify` | `bool` | No | `false` | Disable TLS certificate verification (not recommended for production) | +| `caSecretRef.name` | `string` | No | - | Secret holding a PEM CA bundle used to verify the Warpgate server certificate instead of the system trust store | +| `caSecretRef.key` | `string` | No | `ca.crt` | Key in that Secret | ## Status Fields @@ -120,4 +122,5 @@ The admission webhook applies these defaults if not set: - If the Secret contains the token key, token auth is used regardless of whether username/password keys also exist. - Username/password session auth requires OTP to be disabled on the Warpgate instance. If OTP is enabled, use token auth instead. - The `insecureSkipVerify` flag is provided for development/testing environments with self-signed certificates. Avoid using it in production. + For a private or self-signed CA, point `caSecretRef` at a Secret with the CA certificate instead, which keeps verification on. - Multiple `WarpgateConnection` resources can coexist in the same namespace, each pointing to a different Warpgate instance. Other CRDs select which instance to use via `connectionRef`. diff --git a/docs/crds/warpgate-instance.md b/docs/crds/warpgate-instance.md index 8ab84b5..d9dbbaf 100644 --- a/docs/crds/warpgate-instance.md +++ b/docs/crds/warpgate-instance.md @@ -29,6 +29,8 @@ CRDs like `WarpgateRole` and `WarpgateUser` can reference the deployed instance | `tls.certManager` | `bool` | No | `true` | Enable automatic TLS via cert-manager | | `tls.issuerRef.name` | `string` | No | - | Name of a cert-manager Issuer or ClusterIssuer | | `tls.issuerRef.kind` | `string` | No | - | `Issuer` or `ClusterIssuer` | +| `tls.secretName` | `string` | No | - | Existing TLS Secret (`tls.crt`, `tls.key`, optional `ca.crt`) served by Warpgate | +| `tls.verifyConnection` | `bool` | No | unset | TLS verification for the auto-created `WarpgateConnection`. See notes | | `resources` | `ResourceRequirements` | No | - | CPU/memory requests and limits for the Warpgate container | | `nodeSelector` | `map[string]string` | No | - | Node selector constraints | | `tolerations` | `[]Toleration` | No | - | Scheduling tolerations | @@ -171,6 +173,14 @@ spec: CR in the same namespace, configured to talk to the deployed instance's internal Service URL. The connection name is stored in `status.connectionRef`. Other CRDs can reference it via `connectionRef` to manage resources on this instance. +- **Connection TLS verification:** When `tls.secretName` is set, the auto-created connection verifies Warpgate's + certificate against that Secret's `ca.crt` (or `tls.crt` when there is no `ca.crt`) through `caSecretRef`. The + certificate must then be valid for `-http..svc`. Without `tls.secretName` the pod serves a + self-signed certificate it generates itself, which the operator never sees, so the connection skips verification. + Set `tls.verifyConnection: true` to refuse that (reconciliation fails until `tls.secretName` is set), or `false` to + skip verification even with a TLS Secret, for example when its certificate doesn't cover the in-cluster Service name. + Note that `tls.certManager` on its own does not change the served certificate; to serve and verify the + cert-manager certificate, also set `tls.secretName: -tls`. - **Storage:** Warpgate stores its database and configuration in `/data`. The operator provisions a PVC via the StatefulSet's `volumeClaimTemplates`. Scaling down to zero replicas does not delete the PVC -- data persists across restarts. diff --git a/internal/controller/connection_tls_test.go b/internal/controller/connection_tls_test.go new file mode 100644 index 0000000..ab751c1 --- /dev/null +++ b/internal/controller/connection_tls_test.go @@ -0,0 +1,258 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller + +import ( + "encoding/pem" + "net/http" + "net/http/httptest" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/scheme" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/reconcile" + + warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" +) + +// deleteIgnoringFinalizers removes test objects that may carry a finalizer. +func deleteIgnoringFinalizers(objs ...client.Object) { + for _, obj := range objs { + if err := k8sClient.Get(ctx, client.ObjectKeyFromObject(obj), obj); err != nil { + continue + } + obj.SetFinalizers(nil) + _ = k8sClient.Update(ctx, obj) + _ = k8sClient.Delete(ctx, obj) + } +} + +var _ = Describe("Connection TLS verification", func() { + boolPtr := func(b bool) *bool { return &b } + + Context("WarpgateConnection with caSecretRef", func() { + const ( + authSecret = "conntls-auth" + caSecret = "conntls-ca" + connName = "conntls-conn" + ) + + var ( + srv *httptest.Server + reconciler *WarpgateConnectionReconciler + ) + + BeforeEach(func() { + srv = httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`[]`)) + })) + reconciler = &WarpgateConnectionReconciler{Client: k8sClient, Scheme: k8sClient.Scheme()} + Expect(k8sClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: authSecret, Namespace: testNamespace}, + Data: map[string][]byte{"token": []byte("t")}, + })).To(Succeed()) + }) + + AfterEach(func() { + srv.Close() + deleteIgnoringFinalizers( + &warpgatev1alpha1.WarpgateConnection{ObjectMeta: metav1.ObjectMeta{Name: connName, Namespace: testNamespace}}, + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: authSecret, Namespace: testNamespace}}, + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: caSecret, Namespace: testNamespace}}, + ) + }) + + createConn := func(caData []byte) types.NamespacedName { + if caData != nil { + Expect(k8sClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: caSecret, Namespace: testNamespace}, + Data: map[string][]byte{"ca.crt": caData}, + })).To(Succeed()) + } + Expect(k8sClient.Create(ctx, &warpgatev1alpha1.WarpgateConnection{ + ObjectMeta: metav1.ObjectMeta{Name: connName, Namespace: testNamespace}, + Spec: warpgatev1alpha1.WarpgateConnectionSpec{ + Host: srv.URL, + AuthSecretRef: warpgatev1alpha1.AuthSecretRef{Name: authSecret}, + CASecretRef: &warpgatev1alpha1.SecretKeyRef{Name: caSecret}, + }, + })).To(Succeed()) + return types.NamespacedName{Name: connName, Namespace: testNamespace} + } + + readyReason := func(nn types.NamespacedName) string { + var conn warpgatev1alpha1.WarpgateConnection + Expect(k8sClient.Get(ctx, nn, &conn)).To(Succeed()) + cond := findReadyCondition(conn.Status.Conditions) + Expect(cond).NotTo(BeNil()) + return cond.Reason + } + + It("connects when the CA signs the server certificate", func() { + caPEM := pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw}) + nn := createConn(caPEM) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + Expect(readyReason(nn)).To(Equal("Connected")) + + _, err = getWarpgateClient(ctx, k8sClient, testNamespace, connName) + Expect(err).NotTo(HaveOccurred()) + }) + + It("fails when the CA secret is missing", func() { + nn := createConn(nil) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + Expect(readyReason(nn)).To(Equal("ConnectionFailed")) + + _, err = getWarpgateClient(ctx, k8sClient, testNamespace, connName) + Expect(err).To(MatchError(ContainSubstring("CA secret"))) + }) + + It("fails when the CA secret holds no PEM certificate", func() { + nn := createConn([]byte("not a certificate")) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: nn}) + Expect(err).NotTo(HaveOccurred()) + Expect(readyReason(nn)).To(Equal("ConnectionFailed")) + }) + }) + + Context("auto-created connection from a WarpgateInstance", func() { + const ( + instName = "conntls-inst" + pwSecret = "conntls-inst-pw" + tlsSecret = "conntls-inst-tls" + connName = instName + "-connection" + authSecret = instName + "-admin-auth" + ) + + var reconciler *WarpgateInstanceReconciler + + BeforeEach(func() { + reconciler = &WarpgateInstanceReconciler{Client: k8sClient, Scheme: scheme.Scheme} + Expect(k8sClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: pwSecret, Namespace: testNamespace}, + Data: map[string][]byte{"password": []byte("pw")}, + })).To(Succeed()) + }) + + AfterEach(func() { + deleteIgnoringFinalizers( + &warpgatev1alpha1.WarpgateConnection{ObjectMeta: metav1.ObjectMeta{Name: connName, Namespace: testNamespace}}, + &warpgatev1alpha1.WarpgateInstance{ObjectMeta: metav1.ObjectMeta{Name: instName, Namespace: testNamespace}}, + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: pwSecret, Namespace: testNamespace}}, + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: tlsSecret, Namespace: testNamespace}}, + &corev1.Secret{ObjectMeta: metav1.ObjectMeta{Name: authSecret, Namespace: testNamespace}}, + ) + }) + + newInstance := func(tls *warpgatev1alpha1.InstanceTLSSpec) *warpgatev1alpha1.WarpgateInstance { + return &warpgatev1alpha1.WarpgateInstance{ + ObjectMeta: metav1.ObjectMeta{Name: instName, Namespace: testNamespace}, + Spec: warpgatev1alpha1.WarpgateInstanceSpec{ + Version: "0.21.1", + AdminPasswordSecretRef: warpgatev1alpha1.SecretKeyRef{Name: pwSecret}, + TLS: tls, + }, + } + } + + createTLSSecret := func(data map[string][]byte) { + Expect(k8sClient.Create(ctx, &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: tlsSecret, Namespace: testNamespace}, + Data: data, + })).To(Succeed()) + } + + It("verifies against ca.crt from tls.secretName", func() { + createTLSSecret(map[string][]byte{"ca.crt": []byte("ca"), "tls.crt": []byte("crt"), "tls.key": []byte("key")}) + inst := newInstance(&warpgatev1alpha1.InstanceTLSSpec{SecretName: tlsSecret, CertManager: boolPtr(false)}) + Expect(k8sClient.Create(ctx, inst)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(inst)}) + Expect(err).NotTo(HaveOccurred()) + + var conn warpgatev1alpha1.WarpgateConnection + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: connName, Namespace: testNamespace}, &conn)).To(Succeed()) + Expect(conn.Spec.InsecureSkipVerify).To(BeFalse()) + Expect(conn.Spec.CASecretRef).To(Equal(&warpgatev1alpha1.SecretKeyRef{Name: tlsSecret, Key: "ca.crt"})) + }) + + It("falls back to tls.crt when the TLS secret has no ca.crt", func() { + createTLSSecret(map[string][]byte{"tls.crt": []byte("crt"), "tls.key": []byte("key")}) + insecure, caRef, err := reconciler.connectionTLS(ctx, newInstance(&warpgatev1alpha1.InstanceTLSSpec{SecretName: tlsSecret})) + Expect(err).NotTo(HaveOccurred()) + Expect(insecure).To(BeFalse()) + Expect(caRef).To(Equal(&warpgatev1alpha1.SecretKeyRef{Name: tlsSecret, Key: "tls.crt"})) + }) + + It("errors when the TLS secret has no certificate at all", func() { + createTLSSecret(map[string][]byte{"tls.key": []byte("key")}) + _, _, err := reconciler.connectionTLS(ctx, newInstance(&warpgatev1alpha1.InstanceTLSSpec{SecretName: tlsSecret})) + Expect(err).To(MatchError(ContainSubstring("neither ca.crt nor tls.crt"))) + }) + + It("errors when the TLS secret does not exist", func() { + _, _, err := reconciler.connectionTLS(ctx, newInstance(&warpgatev1alpha1.InstanceTLSSpec{SecretName: tlsSecret})) + Expect(err).To(HaveOccurred()) + }) + + It("skips verification for the self-generated certificate by default", func() { + for _, tls := range []*warpgatev1alpha1.InstanceTLSSpec{nil, {CertManager: boolPtr(true)}, {CertManager: boolPtr(false)}} { + insecure, caRef, err := reconciler.connectionTLS(ctx, newInstance(tls)) + Expect(err).NotTo(HaveOccurred()) + Expect(insecure).To(BeTrue()) + Expect(caRef).To(BeNil()) + } + }) + + It("skips verification when verifyConnection is false, even with a TLS secret", func() { + insecure, caRef, err := reconciler.connectionTLS(ctx, newInstance(&warpgatev1alpha1.InstanceTLSSpec{ + SecretName: tlsSecret, VerifyConnection: boolPtr(false), + })) + Expect(err).NotTo(HaveOccurred()) + Expect(insecure).To(BeTrue()) + Expect(caRef).To(BeNil()) + }) + + It("refuses to create an unverified connection when verifyConnection is true and no CA is known", func() { + inst := newInstance(&warpgatev1alpha1.InstanceTLSSpec{CertManager: boolPtr(false), VerifyConnection: boolPtr(true)}) + Expect(k8sClient.Create(ctx, inst)).To(Succeed()) + + _, err := reconciler.Reconcile(ctx, reconcile.Request{NamespacedName: client.ObjectKeyFromObject(inst)}) + Expect(err).To(MatchError(ContainSubstring("verifyConnection"))) + + var updated warpgatev1alpha1.WarpgateInstance + Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(inst), &updated)).To(Succeed()) + cond := findReadyCondition(updated.Status.Conditions) + Expect(cond).NotTo(BeNil()) + Expect(cond.Reason).To(Equal("ConnectionFailed")) + + var conn warpgatev1alpha1.WarpgateConnection + err = k8sClient.Get(ctx, types.NamespacedName{Name: connName, Namespace: testNamespace}, &conn) + Expect(err).To(HaveOccurred()) + }) + }) +}) diff --git a/internal/controller/helpers.go b/internal/controller/helpers.go index ab61d3e..ab396b1 100644 --- a/internal/controller/helpers.go +++ b/internal/controller/helpers.go @@ -18,6 +18,7 @@ package controller import ( "context" + "crypto/x509" "fmt" corev1 "k8s.io/api/core/v1" @@ -44,6 +45,11 @@ func getWarpgateClient(ctx context.Context, r client.Reader, namespace, connecti return nil, fmt.Errorf("getting auth secret %q: %w", conn.Spec.AuthSecretRef.Name, err) } + caCert, err := loadConnectionCA(ctx, r, &conn) + if err != nil { + return nil, err + } + tokenKey := conn.Spec.AuthSecretRef.TokenKey if tokenKey == "" { tokenKey = "token" @@ -55,6 +61,7 @@ func getWarpgateClient(ctx context.Context, r client.Reader, namespace, connecti Host: conn.Spec.Host, Token: string(token), InsecureSkipVerify: conn.Spec.InsecureSkipVerify, + CACert: caCert, }), nil } @@ -83,5 +90,28 @@ func getWarpgateClient(ctx context.Context, r client.Reader, namespace, connecti Username: string(username), Password: string(password), InsecureSkipVerify: conn.Spec.InsecureSkipVerify, + CACert: caCert, }), nil } + +// loadConnectionCA returns the PEM CA bundle referenced by conn.spec.caSecretRef, +// or nil when there is none or verification is disabled anyway. +func loadConnectionCA(ctx context.Context, r client.Reader, conn *warpgatev1alpha1.WarpgateConnection) ([]byte, error) { + ref := conn.Spec.CASecretRef + if ref == nil || conn.Spec.InsecureSkipVerify { + return nil, nil + } + key := ref.Key + if key == "" { + key = "ca.crt" + } + var secret corev1.Secret + if err := r.Get(ctx, types.NamespacedName{Name: ref.Name, Namespace: conn.Namespace}, &secret); err != nil { + return nil, fmt.Errorf("getting CA secret %q: %w", ref.Name, err) + } + pem := secret.Data[key] + if !x509.NewCertPool().AppendCertsFromPEM(pem) { + return nil, fmt.Errorf("key %q in CA secret %q does not contain a PEM certificate", key, ref.Name) + } + return pem, nil +} diff --git a/internal/controller/warpgateconnection_controller.go b/internal/controller/warpgateconnection_controller.go index c158e0c..f172874 100644 --- a/internal/controller/warpgateconnection_controller.go +++ b/internal/controller/warpgateconnection_controller.go @@ -125,6 +125,11 @@ func (r *WarpgateConnectionReconciler) buildClient(ctx context.Context, conn *wa return nil, fmt.Errorf("getting auth secret %q: %w", conn.Spec.AuthSecretRef.Name, err) } + caCert, err := loadConnectionCA(ctx, r.Client, conn) + if err != nil { + return nil, err + } + tokenKey := conn.Spec.AuthSecretRef.TokenKey if tokenKey == "" { tokenKey = "token" @@ -136,6 +141,7 @@ func (r *WarpgateConnectionReconciler) buildClient(ctx context.Context, conn *wa Host: conn.Spec.Host, Token: string(token), InsecureSkipVerify: conn.Spec.InsecureSkipVerify, + CACert: caCert, }), nil } @@ -164,6 +170,7 @@ func (r *WarpgateConnectionReconciler) buildClient(ctx context.Context, conn *wa Username: string(username), Password: string(password), InsecureSkipVerify: conn.Spec.InsecureSkipVerify, + CACert: caCert, }), nil } diff --git a/internal/controller/warpgateinstance_controller.go b/internal/controller/warpgateinstance_controller.go index d7293d6..63a7fa6 100644 --- a/internal/controller/warpgateinstance_controller.go +++ b/internal/controller/warpgateinstance_controller.go @@ -1155,6 +1155,11 @@ func (r *WarpgateInstanceReconciler) ensureWarpgateConnection(ctx context.Contex return err } + insecure, caRef, err := r.connectionTLS(ctx, inst) + if err != nil { + return err + } + // Create/update the WarpgateConnection CR. host := fmt.Sprintf("https://%s-http.%s.svc:%d", inst.Name, inst.Namespace, instanceHTTPPort(inst)) @@ -1172,7 +1177,8 @@ func (r *WarpgateInstanceReconciler) ensureWarpgateConnection(ctx context.Contex conn.Spec = warpgatev1alpha1.WarpgateConnectionSpec{ Host: host, AuthSecretRef: warpgatev1alpha1.AuthSecretRef{Name: authSecretName}, - InsecureSkipVerify: true, // self-signed cert within cluster + InsecureSkipVerify: insecure, + CASecretRef: caRef, } return nil }) @@ -1184,6 +1190,43 @@ func (r *WarpgateInstanceReconciler) ensureWarpgateConnection(ctx context.Contex return nil } +// connectionTLS decides how the auto-created WarpgateConnection verifies the +// instance. With tls.secretName the operator knows the served certificate and +// verifies against it (ca.crt, falling back to tls.crt). Otherwise the pod +// serves a self-signed certificate generated at startup that the operator never +// sees, so verification is skipped unless tls.verifyConnection demands it. +func (r *WarpgateInstanceReconciler) connectionTLS( + ctx context.Context, inst *warpgatev1alpha1.WarpgateInstance, +) (insecure bool, caRef *warpgatev1alpha1.SecretKeyRef, err error) { + var verify *bool + if inst.Spec.TLS != nil { + verify = inst.Spec.TLS.VerifyConnection + } + if verify != nil && !*verify { + return true, nil, nil + } + + if tlsSecretProvided(inst) { + name := inst.Spec.TLS.SecretName + var secret corev1.Secret + if err := r.Get(ctx, types.NamespacedName{Name: name, Namespace: inst.Namespace}, &secret); err != nil { + return false, nil, fmt.Errorf("getting TLS secret %q for connection verification: %w", name, err) + } + for _, key := range []string{"ca.crt", corev1.TLSCertKey} { + if len(secret.Data[key]) > 0 { + return false, &warpgatev1alpha1.SecretKeyRef{Name: name, Key: key}, nil + } + } + return false, nil, fmt.Errorf("TLS secret %q has neither ca.crt nor tls.crt to verify the connection against", name) + } + + if verify != nil && *verify { + return false, nil, fmt.Errorf("tls.verifyConnection is true but there is no CA to verify against; set tls.secretName") + } + // Self-signed certificate generated inside the pod. + return true, nil, nil +} + // --------------------------------------------------------------------------- // 10. Status update // --------------------------------------------------------------------------- diff --git a/internal/warpgate/client.go b/internal/warpgate/client.go index 68fe8b6..d30e85c 100644 --- a/internal/warpgate/client.go +++ b/internal/warpgate/client.go @@ -3,6 +3,7 @@ package warpgate import ( "bytes" "crypto/tls" + "crypto/x509" "encoding/json" "fmt" "io" @@ -20,6 +21,9 @@ type Config struct { Username string // For session auth fallback Password string // For session auth fallback InsecureSkipVerify bool + // CACert is a PEM bundle of CAs trusted for the Warpgate server certificate. + // When set (and InsecureSkipVerify is false), it replaces the system roots. + CACert []byte } // APIError represents an error response from the Warpgate API. @@ -52,10 +56,18 @@ func NewClient(cfg Config) *Client { jar, _ := cookiejar.New(nil) transport := &http.Transport{} - if cfg.InsecureSkipVerify { + switch { + case cfg.InsecureSkipVerify: transport.TLSClientConfig = &tls.Config{ InsecureSkipVerify: true, // #nosec G402 -- user-configured InsecureSkipVerify } + case len(cfg.CACert) > 0: + pool := x509.NewCertPool() + pool.AppendCertsFromPEM(cfg.CACert) + transport.TLSClientConfig = &tls.Config{ + RootCAs: pool, + MinVersion: tls.VersionTLS12, + } } c := &Client{ diff --git a/internal/warpgate/client_test.go b/internal/warpgate/client_test.go index 06873a2..cd48e17 100644 --- a/internal/warpgate/client_test.go +++ b/internal/warpgate/client_test.go @@ -1,13 +1,21 @@ package warpgate import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/x509" + "crypto/x509/pkix" "encoding/json" + "encoding/pem" "errors" "math" + "math/big" "net/http" "net/http/httptest" "strings" "testing" + "time" ) func TestNewClient(t *testing.T) { @@ -524,3 +532,79 @@ func TestAPIErrorString(t *testing.T) { t.Errorf("expected %q, got %q", expected, err.Error()) } } + +func tlsServerCAPEM(srv *httptest.Server) []byte { + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: srv.Certificate().Raw}) +} + +func TestCACertVerifiesServer(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`[]`)) + })) + defer srv.Close() + + c := NewClient(Config{Host: srv.URL, Token: "t", CACert: tlsServerCAPEM(srv)}) + tr := c.httpClient.Transport.(*http.Transport) + if tr.TLSClientConfig == nil || tr.TLSClientConfig.RootCAs == nil { + t.Fatal("expected RootCAs to be set from CACert") + } + if tr.TLSClientConfig.InsecureSkipVerify { + t.Fatal("CACert must not disable verification") + } + var out []any + if err := c.Get("/roles", &out); err != nil { + t.Fatalf("expected request to succeed against the trusted CA, got: %v", err) + } +} + +func TestCACertRejectsOtherServer(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer srv.Close() + + // Trust an unrelated self-signed CA that did not sign the server's cert. + c := NewClient(Config{Host: srv.URL, Token: "t", CACert: selfSignedPEM(t)}) + if err := c.Get("/roles", nil); err == nil { + t.Fatal("expected certificate verification to fail for an untrusted server") + } +} + +func TestNoCACertUsesSystemRoots(t *testing.T) { + srv := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) + defer srv.Close() + + c := NewClient(Config{Host: srv.URL, Token: "t"}) + if err := c.Get("/roles", nil); err == nil { + t.Fatal("expected verification against system roots to reject the test server") + } +} + +func TestInsecureSkipVerifyWinsOverCACert(t *testing.T) { + c := NewClient(Config{Host: "https://x", InsecureSkipVerify: true, CACert: selfSignedPEM(t)}) + tr := c.httpClient.Transport.(*http.Transport) + if !tr.TLSClientConfig.InsecureSkipVerify || tr.TLSClientConfig.RootCAs != nil { + t.Fatal("expected InsecureSkipVerify to take precedence over CACert") + } +} + +func selfSignedPEM(t *testing.T) []byte { + t.Helper() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + t.Fatal(err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: "unrelated"}, + NotBefore: time.Now().Add(-time.Hour), + NotAfter: time.Now().Add(time.Hour), + IsCA: true, + BasicConstraintsValid: true, + KeyUsage: x509.KeyUsageCertSign, + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatal(err) + } + return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: der}) +} diff --git a/openspec/specs/connection-management/spec.md b/openspec/specs/connection-management/spec.md index ea4d2f6..e9d04f4 100644 --- a/openspec/specs/connection-management/spec.md +++ b/openspec/specs/connection-management/spec.md @@ -17,6 +17,7 @@ The operator provides a `WarpgateConnection` custom resource with the following - `usernameKey` (optional, default `username`) -- key holding the username for session auth. - `passwordKey` (optional, default `password`) -- key holding the password for session auth. - `insecureSkipVerify` (optional, default `false`) -- disables TLS certificate verification. +- `caSecretRef` (optional) -- Secret key (`name`, `key` defaulting to `ca.crt`) with a PEM CA bundle used as the trust roots for the Warpgate server certificate. Ignored when `insecureSkipVerify` is true. Auth mode is auto-detected: if the Secret contains the token key, bearer auth is used. Otherwise, the operator falls back to username/password session auth. @@ -66,3 +67,15 @@ The controller reads the auth Secret from the same namespace as the `WarpgateCon **Scenarios:** - **Given** a `WarpgateConnection` in namespace `team-a` referencing Secret `wg-token` **When** the controller reads the Secret **Then** it looks up `team-a/wg-token`, not a cluster-wide search. + +### REQ-CONN-006: Custom CA Trust + +**Status:** ADDED + +When `caSecretRef` is set, the operator reads the PEM bundle from the Secret and uses it as the HTTP client's `RootCAs`, so private or self-signed CAs can be trusted without disabling verification. A missing Secret or a key without a PEM certificate sets `Ready=False` with reason `ConnectionFailed`. + +**Scenarios:** + +- **Given** a connection whose `caSecretRef` holds the CA that signed Warpgate's certificate **When** the controller reconciles **Then** it connects and sets `Ready=True`. +- **Given** a connection whose `caSecretRef` points at a missing Secret or non-PEM data **When** the controller reconciles **Then** it sets `Ready=False` with reason `ConnectionFailed`. + diff --git a/openspec/specs/instance-management/spec.md b/openspec/specs/instance-management/spec.md index ebe17fb..992c437 100644 --- a/openspec/specs/instance-management/spec.md +++ b/openspec/specs/instance-management/spec.md @@ -21,7 +21,7 @@ The operator provides a `WarpgateInstance` custom resource with the following sp - `mysql` (optional) -- MySQL proxy listener: `enabled`, `port`. - `postgresql` (optional) -- PostgreSQL proxy listener: `enabled`, `port`. - `storage` (optional) -- PVC configuration: `size` (default `1Gi`), `storageClassName`. -- `tls` (optional) -- TLS configuration: `certManager` (default `true`), `issuerRef` with `name` and `kind`. +- `tls` (optional) -- TLS configuration: `certManager` (default `true`), `issuerRef` with `name` and `kind`, `secretName`, and `verifyConnection`. - `resources` (optional) -- CPU/memory requests and limits. - `nodeSelector` (optional) -- node scheduling constraints. - `tolerations` (optional) -- scheduling tolerations. @@ -128,3 +128,16 @@ When an external database is configured (`databaseURLSecretRef` or the deprecate - **Given** a `WarpgateInstance` with only `databaseURL` **When** it is admitted **Then** the webhook returns a deprecation warning and the instance keeps working. - **Given** an `externalHost` containing a newline and extra YAML keys **When** the config is generated **Then** it parses back to the exact `externalHost` string and no extra keys appear. - **Given** the operator changes the generated init script **When** it reconciles an existing instance **Then** the pod template hash changes and the Deployment rolls out. + +### REQ-INST-009: Verified Auto-Created Connection + +**Status:** ADDED + +The auto-created `WarpgateConnection` verifies the instance's TLS certificate whenever the operator knows it. With `tls.secretName`, the connection gets `caSecretRef` pointing at that Secret's `ca.crt` (or `tls.crt` if there is no `ca.crt`) and `insecureSkipVerify: false`. Without it, the pod serves a self-signed certificate it generates at startup, so the connection skips verification. `tls.verifyConnection: true` makes the missing CA a reconcile error instead; `tls.verifyConnection: false` always skips verification. + +**Scenarios:** + +- **Given** a `WarpgateInstance` with `tls.secretName` whose Secret has `ca.crt` **When** the controller reconciles **Then** the connection has `caSecretRef: {name: , key: ca.crt}` and verification on. +- **Given** a `WarpgateInstance` without `tls.secretName` **When** the controller reconciles **Then** the connection has `insecureSkipVerify: true`. +- **Given** `tls.verifyConnection: true` and no `tls.secretName` **When** the controller reconciles **Then** no connection is created and `Ready=False` with reason `ConnectionFailed`. + From 55378b13b43feea0842794554d84d7b778c21dcd Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:53:56 +0300 Subject: [PATCH 6/9] docs(openspec): archive security hardening change --- .../2026-10-06-security-hardening/design.md | 38 +++++++++++++++++++ .../2026-10-06-security-hardening/proposal.md | 28 ++++++++++++++ .../2026-10-06-security-hardening/tasks.md | 13 +++++++ 3 files changed, 79 insertions(+) create mode 100644 openspec/changes/archive/2026-10-06-security-hardening/design.md create mode 100644 openspec/changes/archive/2026-10-06-security-hardening/proposal.md create mode 100644 openspec/changes/archive/2026-10-06-security-hardening/tasks.md diff --git a/openspec/changes/archive/2026-10-06-security-hardening/design.md b/openspec/changes/archive/2026-10-06-security-hardening/design.md new file mode 100644 index 0000000..fe19633 --- /dev/null +++ b/openspec/changes/archive/2026-10-06-security-hardening/design.md @@ -0,0 +1,38 @@ +# Design + +## Database URL (init script and ConfigMap) + +Warpgate's config loader only reads the YAML file (no environment source), so `database_url` has to end up in `/data/warpgate.yaml`. The ConfigMap now omits it whenever an external database is configured. The init container gets `DATABASE_URL` (from `databaseURLSecretRef` via `secretKeyRef`, or the deprecated literal) and, after copying the ConfigMap file, deletes any `database_url:` line and appends `database_url: ''` with single quotes doubled. Single-quoted YAML needs no other escaping, and the value only ever flows through `printf`/`sed` as data. + +`--database-url "${DATABASE_URL}"` replaces the interpolated flag in `unattended-setup`, matching how `ADMIN_PASSWORD` was already handled. + +The init script text is folded into the pod template hash. Without that, existing Deployments would keep the old script while the ConfigMap lost `database_url`, and the next restart would fall back to SQLite. + +`databaseURL` keeps working for backward compatibility. The webhook warns that it is deprecated, and that it is ignored when `databaseURLSecretRef` is also set. Rotating the database Secret takes effect on the next pod restart; the operator does not restart pods for Secret changes. + +## Target TLS verify + +`Verify` becomes `*bool` with `+kubebuilder:default=true`, the webhook defaulter sets it for any present `tls` block, and `toWarpgateTLS` maps `nil` to `true`. All three layers agree so objects created before the CRD upgrade (stored without the field) also verify. + +## Password rotation + +Warpgate's API has no update for password credentials. The controller records `/@` in `status.appliedSecretVersion`. When the current value differs (or the resolved user changed), it deletes the old credential and then creates a new one. Delete-first means the old password never outlives the rotation; the cost is a brief window with no password credential. If the delete fails, the old ID is kept and `Ready=False/RotateFailed` is reported. + +The resourceVersion is used instead of a password hash so no derivative of the secret lands in the CR status. Credentials without a recorded version (created by older operator versions) are re-applied once after upgrade. + +A `Watches(&corev1.Secret{}, EnqueueRequestsFromMapFunc(...))` lists credentials in the Secret's namespace and enqueues those whose `passwordSecretRef.name` matches. The manager already caches Secrets for the other controllers, so this adds no new informer. + +## Auto-created connection TLS + +`WarpgateConnection.spec.caSecretRef` points to a PEM bundle used as `RootCAs`. Both client builders (connection controller and shared helper) load it and reject a Secret without a PEM certificate. + +For a `WarpgateInstance`: + +| `tls.secretName` | `tls.verifyConnection` | Result | +|---|---|---| +| set | unset / true | verify with `caSecretRef` (`ca.crt`, else `tls.crt`) | +| set | false | skip verification | +| unset | unset / false | skip verification (pod-generated self-signed cert) | +| unset | true | reconcile error, no connection | + +`tls.certManager` on its own does not mount the cert-manager Secret into the pod (the pod serves Warpgate's own self-signed certificate), so it falls in the "unset" rows. Users who want the cert-manager certificate served and verified set `tls.secretName: -tls`. diff --git a/openspec/changes/archive/2026-10-06-security-hardening/proposal.md b/openspec/changes/archive/2026-10-06-security-hardening/proposal.md new file mode 100644 index 0000000..a30ba30 --- /dev/null +++ b/openspec/changes/archive/2026-10-06-security-hardening/proposal.md @@ -0,0 +1,28 @@ +# Security Hardening + +## Summary + +Fix five security issues in how the operator handles database credentials, TLS verification, and password rotation. + +## Motivation + +An audit of the controllers turned up these problems on `main`: + +1. `WarpgateInstance.spec.databaseURL` was interpolated into the init container's `sh -c` script, so a crafted value ran arbitrary commands. +2. The same URL, usually with a database password, was written in plain text into the generated `warpgate.yaml` ConfigMap. +3. A `WarpgateTarget` `tls` block that set only `mode` was sent to Warpgate with `verify: false`, silently disabling certificate checks. +4. Rotating the Secret behind a `WarpgatePasswordCredential` never reached Warpgate, so the old password stayed valid. +5. The `WarpgateConnection` auto-created for a `WarpgateInstance` always set `insecureSkipVerify: true`, even when the instance served a certificate the operator could verify. + +## What Changes + +- Init script reads the database URL from `DATABASE_URL` instead of embedding it. +- New `WarpgateInstance.spec.databaseURLSecretRef`; `databaseURL` is deprecated. The URL is no longer written to the ConfigMap; the init container appends it to `/data/warpgate.yaml`. `externalHost` goes through a YAML marshaller. +- `TLSConfigSpec.verify` becomes `*bool` defaulting to `true`. +- The password credential controller watches Secrets and replaces the Warpgate credential when the Secret changes, tracking `status.appliedSecretVersion`. +- New `WarpgateConnection.spec.caSecretRef` and `WarpgateInstance.spec.tls.verifyConnection`; the auto-created connection verifies against `tls.secretName` when set. + +## Breaking Changes + +- Target `tls` blocks without `verify` now verify certificates. +- Instances with `tls.secretName` get a verifying connection; the certificate must cover `-http..svc` (or set `tls.verifyConnection: false`). diff --git a/openspec/changes/archive/2026-10-06-security-hardening/tasks.md b/openspec/changes/archive/2026-10-06-security-hardening/tasks.md new file mode 100644 index 0000000..674aefc --- /dev/null +++ b/openspec/changes/archive/2026-10-06-security-hardening/tasks.md @@ -0,0 +1,13 @@ +# Tasks + +- [x] Pass the database URL to the init script through `DATABASE_URL` +- [x] Add `databaseURLSecretRef`, deprecate `databaseURL`, keep the URL out of the ConfigMap +- [x] Emit `externalHost` through a YAML marshaller +- [x] Fold the init script into the pod template hash +- [x] Make `TLSConfigSpec.verify` a `*bool` defaulting to `true` (CRD, webhook, converter) +- [x] Watch Secrets from the password credential controller and replace credentials on rotation +- [x] Add `WarpgateConnection.spec.caSecretRef` and wire it into the HTTP client +- [x] Add `WarpgateInstance.spec.tls.verifyConnection` and verify the auto-created connection when the CA is known +- [x] Unit and envtest coverage for each fix +- [x] Regenerate CRDs, deepcopy and Helm chart CRDs +- [x] Update CRD docs and living specs From 6ece838bd6c0022a67fe737650b5a89f4f6fa52e Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:06:35 +0300 Subject: [PATCH 7/9] fix(instance): don't hash the raw database URL into the rollout annotation The pod template hash includes spec.databaseURL, which usually embeds a password, and the hash is visible to anyone who can read pods. Derive the database URL's contribution with PBKDF2 so the annotation still changes on a new URL but can't be used to cheaply test password guesses. --- .../database_url_fingerprint_test.go | 43 +++++++++++++++++++ .../controller/warpgateinstance_controller.go | 25 ++++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 internal/controller/database_url_fingerprint_test.go diff --git a/internal/controller/database_url_fingerprint_test.go b/internal/controller/database_url_fingerprint_test.go new file mode 100644 index 0000000..a48dd87 --- /dev/null +++ b/internal/controller/database_url_fingerprint_test.go @@ -0,0 +1,43 @@ +package controller + +import ( + "crypto/sha256" + "strings" + "testing" + + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + warpgatev1alpha1 "github.com/thereisnotime/warpgate-operator/api/v1alpha1" +) + +func TestDatabaseURLFingerprint(t *testing.T) { + inst := &warpgatev1alpha1.WarpgateInstance{ObjectMeta: metav1.ObjectMeta{UID: "uid-1"}} + if got := databaseURLFingerprint(inst); got != nil { + t.Fatalf("expected nil fingerprint without a database URL, got %x", got) + } + + inst.Spec.DatabaseURL = "postgres://user:hunter2@db:5432/warpgate" + a := databaseURLFingerprint(inst) + if len(a) != 32 { + t.Fatalf("expected 32-byte fingerprint, got %d", len(a)) + } + raw := sha256.Sum256([]byte(inst.Spec.DatabaseURL)) + if string(a) == string(raw[:]) { + t.Fatal("fingerprint must not be a plain SHA-256 of the URL") + } + if string(a) != string(databaseURLFingerprint(inst)) { + t.Fatal("fingerprint must be deterministic") + } + + before := configHash(inst) + inst.Spec.DatabaseURL = "postgres://user:rotated@db:5432/warpgate" + if databaseURLFingerprint(inst) == nil || string(databaseURLFingerprint(inst)) == string(a) { + t.Fatal("fingerprint must change when the URL changes") + } + if after := configHash(inst); after == before { + t.Fatal("configHash must change when the database URL changes") + } + if strings.Contains(configHash(inst), "rotated") { + t.Fatal("configHash must not contain the URL") + } +} diff --git a/internal/controller/warpgateinstance_controller.go b/internal/controller/warpgateinstance_controller.go index 63a7fa6..3bce29a 100644 --- a/internal/controller/warpgateinstance_controller.go +++ b/internal/controller/warpgateinstance_controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "crypto/pbkdf2" "crypto/sha256" "fmt" "strings" @@ -368,7 +369,7 @@ func configHash(inst *warpgatev1alpha1.WarpgateInstance) string { kubernetesEnabled(inst))) h.Write([]byte(inst.Spec.ExternalHost)) h.Write([]byte(inst.Spec.ConfigOverride)) - h.Write([]byte(inst.Spec.DatabaseURL)) + h.Write(databaseURLFingerprint(inst)) if databaseURLFromSecret(inst) { h.Write([]byte(inst.Spec.DatabaseURLSecretRef.Name + "/" + databaseURLKey(inst))) } @@ -376,6 +377,28 @@ func configHash(inst *warpgatev1alpha1.WarpgateInstance) string { return fmt.Sprintf("%x", h.Sum(nil))[:12] } +// databaseURLFingerprintIterations makes guessing a database password from the +// published rollout hash expensive; the URL usually embeds a credential. +const databaseURLFingerprintIterations = 100_000 + +// databaseURLFingerprint derives a change-detection value for spec.databaseURL +// with PBKDF2 instead of feeding the raw URL into the fast rollout hash, which +// ends up in a pod template annotation anyone with pod read access can see. +func databaseURLFingerprint(inst *warpgatev1alpha1.WarpgateInstance) []byte { + if inst.Spec.DatabaseURL == "" { + return nil + } + key, err := pbkdf2.Key(sha256.New, inst.Spec.DatabaseURL, + []byte("warpgate-operator/database-url/"+string(inst.UID)), + databaseURLFingerprintIterations, 32) + if err != nil { + // Only fails for invalid parameters; still change on URL changes + // without exposing the raw value. + return []byte("database-url-set") + } + return key +} + // --------------------------------------------------------------------------- // 4. ConfigMap — warpgate.yaml // --------------------------------------------------------------------------- From 734e2e4e534d98c6c0d72df198c2f4a511ac31ef Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:13:22 +0300 Subject: [PATCH 8/9] fix(instance): use a non-cryptographic checksum for the rollout key The pod rollout key only detects changes to the config and the generated init script, which references secrets by env var name only. FNV-1a makes that intent explicit instead of reading as password hashing. --- internal/controller/warpgateinstance_controller.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/controller/warpgateinstance_controller.go b/internal/controller/warpgateinstance_controller.go index 3bce29a..f886de5 100644 --- a/internal/controller/warpgateinstance_controller.go +++ b/internal/controller/warpgateinstance_controller.go @@ -21,6 +21,7 @@ import ( "crypto/pbkdf2" "crypto/sha256" "fmt" + "hash/fnv" "strings" "time" @@ -759,8 +760,13 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp } // Fold the init script into the rollout hash so pods pick up changes to - // the generated script (not just to the CR spec). - hash := fmt.Sprintf("%x", sha256.Sum256([]byte(configHash(inst)+initScript)))[:12] + // the generated script (not just to the CR spec). This is a change-detection + // key, not a security hash: the script only references secrets by env var + // name and configHash already derives the database URL with PBKDF2. + rollout := fnv.New64a() + rollout.Write([]byte(configHash(inst))) + rollout.Write([]byte(initScript)) + hash := fmt.Sprintf("%016x", rollout.Sum64())[:12] // Deployment strategy — default Recreate for RWO PVC compatibility. strategy := appsv1.DeploymentStrategy{ From d78643a649146a7f4515c0fc0aa5d2f43f16a61b Mon Sep 17 00:00:00 2001 From: thereisnotime <37583483+thereisnotime@users.noreply.github.com> Date: Wed, 7 Oct 2026 00:20:10 +0300 Subject: [PATCH 9/9] fix(instance): handle rollout checksum write errors explicitly --- internal/controller/warpgateinstance_controller.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/controller/warpgateinstance_controller.go b/internal/controller/warpgateinstance_controller.go index f886de5..633ab0e 100644 --- a/internal/controller/warpgateinstance_controller.go +++ b/internal/controller/warpgateinstance_controller.go @@ -764,8 +764,8 @@ func (r *WarpgateInstanceReconciler) buildDeployment(inst *warpgatev1alpha1.Warp // key, not a security hash: the script only references secrets by env var // name and configHash already derives the database URL with PBKDF2. rollout := fnv.New64a() - rollout.Write([]byte(configHash(inst))) - rollout.Write([]byte(initScript)) + _, _ = rollout.Write([]byte(configHash(inst))) // hash writes never fail + _, _ = rollout.Write([]byte(initScript)) hash := fmt.Sprintf("%016x", rollout.Sum64())[:12] // Deployment strategy — default Recreate for RWO PVC compatibility.