From 3910f777809e9b1e179ad92b369c8d94728ef43c Mon Sep 17 00:00:00 2001 From: Daniel Panzella Date: Wed, 19 Aug 2026 21:39:02 -0700 Subject: [PATCH 1/3] feat(api): Accept a literal value or secret reference for connection fields External-connection fields (MySQL, Redis, ClickHouse, Kafka, object store) and OIDC previously required a Secret reference for every field, even non-secret values like host, port, bucket, region, endpoint, and the OIDC issuer URL. Introduce a ValueOrSecret envelope (a literal 'value' or 'valueFrom.secretKeyRef') so any field can be supplied inline or from a Secret. Existing CRs using the legacy {name, key} shape keep working: a defaulting webhook normalizes them into valueFrom on admission (the legacy fields are deprecated and removed at v2 GA). ProxyValue folds onto the shared type. Secret-bearing fields (password, sslKey, secretKey, clientSecret, assembled URLs) carry a masq:"secret" tag and the operator log handler redacts them via github.com/m-mizutani/masq. The manifest custom-resource env resolver is now union-aware. Validated with make lint / make test and westest local-kind-ingress + local-kind-external. Co-Authored-By: Claude Opus 4.8 --- api/v1/weightsandbiases_conversion_mapping.go | 78 +- api/v1/weightsandbiases_conversion_test.go | 138 +- api/v2/weightsandbiases_types.go | 302 ++- api/v2/zz_generated.deepcopy.go | 89 +- .../apps.wandb.com_weightsandbiases.yaml | 1742 ++++++++++++++--- .../secret_or_value_connection_fields.md | 670 +++++++ go.mod | 1 + go.sum | 4 + hack/tilt/wandbcr/main.go | 41 +- hack/tilt/wandbcr/main_test.go | 53 +- .../infra/external/clickhouse/clickhouse.go | 21 +- internal/controller/infra/external/common.go | 34 + .../controller/infra/external/mysql/mysql.go | 27 +- .../infra/external/mysql/mysql_test.go | 7 +- .../infra/external/objectstore/objectstore.go | 5 +- .../external/objectstore/objectstore_test.go | 77 +- .../controller/infra/external/redis/redis.go | 19 +- .../infra/external/redis/redis_test.go | 7 +- .../infra/managed/clickhouse/altinity/conn.go | 15 +- .../infra/managed/kafka/bufstream/conn.go | 9 +- .../managed/kafka/bufstream/write_test.go | 7 +- .../infra/managed/mysql/moco/conn.go | 13 +- .../infra/managed/redis/opstree/conn.go | 7 +- .../infra/objectstore/resolve_test.go | 11 +- .../controller/infra/objectstore/secret.go | 37 +- .../infra/objectstore/secret_test.go | 33 +- internal/controller/reconciler/custom_ca.go | 43 +- .../controller/reconciler/custom_ca_test.go | 34 +- internal/controller/reconciler/kafka.go | 5 +- .../controller/reconciler/migrate_legacy.go | 37 +- .../reconciler/migrate_legacy_test.go | 201 +- .../controller/reconciler/oidc_env_test.go | 6 +- internal/controller/reconciler/pods.go | 32 +- .../reconciler/pods_instance_test.go | 7 +- internal/controller/reconciler/proxy_env.go | 19 +- .../controller/reconciler/proxy_env_test.go | 10 +- .../controller/reconciler/reconcile_v2.go | 26 +- ...htsandbiases_controller_networking_test.go | 10 +- .../weightsandbiases_controller_test.go | 24 +- .../apps.wandb.com_weightsandbiases.yaml | 1742 ++++++++++++++--- internal/logx/handler.go | 11 +- internal/logx/pretty.go | 17 +- internal/logx/redact.go | 26 + internal/logx/redact_test.go | 33 + .../webhook/v2/weightsandbiases_proxy_test.go | 20 +- .../webhook/v2/weightsandbiases_webhook.go | 148 +- .../v2/weightsandbiases_webhook_test.go | 14 +- pkg/utils/connection_secrets.go | 13 + 48 files changed, 4596 insertions(+), 1329 deletions(-) create mode 100644 docs/design/wandb_v2/secret_or_value_connection_fields.md create mode 100644 internal/logx/redact.go create mode 100644 internal/logx/redact_test.go diff --git a/api/v1/weightsandbiases_conversion_mapping.go b/api/v1/weightsandbiases_conversion_mapping.go index 1a581361..4fa45e75 100644 --- a/api/v1/weightsandbiases_conversion_mapping.go +++ b/api/v1/weightsandbiases_conversion_mapping.go @@ -407,14 +407,8 @@ func mapBucket(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiases) e if secretKeyName == "" { secretKeyName = defaultBucketSecretKeyName } - conn.AccessKey = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: accessKeyName, - } - conn.SecretKey = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: secretKeyName, - } + conn.AccessKey = appsv2.ValueFromSecret(name, accessKeyName, false) + conn.SecretKey = appsv2.ValueFromSecret(name, secretKeyName, false) } } @@ -455,12 +449,12 @@ var mysqlFields = []struct { v1Key string setRef func(*appsv2.MysqlConnection, corev1.SecretKeySelector) }{ - {"host", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Host = s }}, - {"port", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Port = s }}, - {"database", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Database = s }}, - {"user", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Username = s }}, - {"password", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Password = s }}, - {"caCert", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.SslCa = s }}, + {"host", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Host = appsv2.ValueFromSelector(s) }}, + {"port", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Port = appsv2.ValueFromSelector(s) }}, + {"database", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Database = appsv2.ValueFromSelector(s) }}, + {"user", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Username = appsv2.ValueFromSelector(s) }}, + {"password", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.Password = appsv2.ValueFromSelector(s) }}, + {"caCert", func(c *appsv2.MysqlConnection, s corev1.SecretKeySelector) { c.SslCa = appsv2.ValueFromSelector(s) }}, } // mapMySQL routes valueFrom-shaped fields to externalMysql.*, scalars to @@ -500,16 +494,13 @@ func mapMySQL(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiases) er return fmt.Errorf("spec.values.global.mysql.passwordSecret: %w", err) } else if ok { name, _, _ := unstructured.NestedString(ps, "name") - alreadyHasPassword := conn != nil && conn.Password.Name != "" + alreadyHasPassword := conn != nil && conn.Password.SecretKeyRef() != nil if name != "" && !alreadyHasPassword { key, _, _ := unstructured.NestedString(ps, "passwordKey") if key == "" { key = defaultMySQLPasswordSecretKey } - conn.Password = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } + conn.Password = appsv2.ValueFromSecret(name, key, false) delete(remaining, "password") } } @@ -530,11 +521,11 @@ var clickHouseFields = []struct { v1Key string setRef func(*appsv2.ClickHouseConnection, corev1.SecretKeySelector) }{ - {"host", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Host = s }}, - {"port", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.HTTPPort = s }}, - {"database", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Database = s }}, - {"user", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Username = s }}, - {"password", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Password = s }}, + {"host", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Host = appsv2.ValueFromSelector(s) }}, + {"port", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.HTTPPort = appsv2.ValueFromSelector(s) }}, + {"database", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Database = appsv2.ValueFromSelector(s) }}, + {"user", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Username = appsv2.ValueFromSelector(s) }}, + {"password", func(c *appsv2.ClickHouseConnection, s corev1.SecretKeySelector) { c.Password = appsv2.ValueFromSelector(s) }}, } // mapClickHouse routes v1 global.clickhouse to externalClickhouse (like @@ -578,7 +569,7 @@ func mapClickHouse(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiase if err != nil { return fmt.Errorf("spec.values.global.clickhouse.passwordSecret.name: %w", err) } - alreadyHasPassword := conn.Password.Name != "" + alreadyHasPassword := conn.Password.SecretKeyRef() != nil if name != "" && !alreadyHasPassword { key, _, err := unstructured.NestedString(ps, "passwordKey") if err != nil { @@ -587,10 +578,7 @@ func mapClickHouse(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiase if key == "" { key = defaultClickHousePasswordSecretKey } - conn.Password = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } + conn.Password = appsv2.ValueFromSecret(name, key, false) delete(remaining, "password") sawField = true } @@ -615,10 +603,10 @@ var redisFields = []struct { v1Key string setRef func(*appsv2.RedisConnection, corev1.SecretKeySelector) }{ - {"host", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.Host = s }}, - {"port", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.Port = s }}, - {"password", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.Password = s }}, - {"caCert", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.SslCa = s }}, + {"host", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.Host = appsv2.ValueFromSelector(s) }}, + {"port", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.Port = appsv2.ValueFromSelector(s) }}, + {"password", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.Password = appsv2.ValueFromSelector(s) }}, + {"caCert", func(c *appsv2.RedisConnection, s corev1.SecretKeySelector) { c.SslCa = appsv2.ValueFromSelector(s) }}, } // mapRedis routes valueFrom-shaped fields to externalRedis.*, scalars to @@ -670,7 +658,7 @@ func mapRedis(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiases) er return fmt.Errorf("spec.values.global.redis.%s.tls: %w", parent, classifyErr) } if ref != nil { - conn.Tls = *ref + conn.Tls = appsv2.ValueFromSelector(*ref) break } if literal != "" { @@ -683,16 +671,13 @@ func mapRedis(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiases) er return fmt.Errorf("spec.values.global.redis.secret: %w", err) } else if ok { name, _, _ := unstructured.NestedString(sec, "secretName") - alreadyHasPassword := conn != nil && conn.Password.Name != "" + alreadyHasPassword := conn != nil && conn.Password.SecretKeyRef() != nil if name != "" && !alreadyHasPassword { key, _, _ := unstructured.NestedString(sec, "secretKey") if key == "" { key = defaultRedisPasswordSecretKey } - conn.Password = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } + conn.Password = appsv2.ValueFromSecret(name, key, false) delete(remaining, "password") } } @@ -713,10 +698,10 @@ var oidcFields = []struct { v1Key string setRef func(*appsv2.OidcSpec, corev1.SecretKeySelector) }{ - {"clientId", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.ClientId = s }}, - {"secret", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.ClientSecret = s }}, - {"authMethod", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.AuthMethod = s }}, - {"issuer", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.IssuerUrl = s }}, + {"clientId", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.ClientId = appsv2.ValueFromSelector(s) }}, + {"secret", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.ClientSecret = appsv2.ValueFromSelector(s) }}, + {"authMethod", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.AuthMethod = appsv2.ValueFromSelector(s) }}, + {"issuer", func(o *appsv2.OidcSpec, s corev1.SecretKeySelector) { o.IssuerUrl = appsv2.ValueFromSelector(s) }}, } // mapOIDC routes valueFrom-shaped fields to spec.wandb.oidc.*, scalars to @@ -755,16 +740,13 @@ func mapOIDC(globalMap map[string]interface{}, dst *appsv2.WeightsAndBiases) err return fmt.Errorf("spec.values.global.auth.oidc.oidcSecret: %w", err) } else if ok { name, _, _ := unstructured.NestedString(os, "name") - alreadyHasClientSecret := oidc.ClientSecret.Name != "" + alreadyHasClientSecret := oidc.ClientSecret.SecretKeyRef() != nil if name != "" && !alreadyHasClientSecret { key, _, _ := unstructured.NestedString(os, "secretKey") if key == "" { key = defaultOIDCClientSecretKey } - oidc.ClientSecret = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } + oidc.ClientSecret = appsv2.ValueFromSecret(name, key, false) delete(remaining, "secret") } } diff --git a/api/v1/weightsandbiases_conversion_test.go b/api/v1/weightsandbiases_conversion_test.go index a352d62d..64e802e9 100644 --- a/api/v1/weightsandbiases_conversion_test.go +++ b/api/v1/weightsandbiases_conversion_test.go @@ -625,8 +625,8 @@ func TestConvertTo_OIDCAllLiterals(t *testing.T) { require.Equal(t, "https://example.com", decoded["issuer"]) require.NotContains(t, decoded, "oidcSecret") - require.Empty(t, dst.Spec.Wandb.OIDC.ClientId.Name, "no ref-shaped values, so spec.wandb.oidc stays unset") - require.Empty(t, dst.Spec.Wandb.OIDC.ClientSecret.Name) + require.Nil(t, dst.Spec.Wandb.OIDC.ClientId.SecretKeyRef(), "no ref-shaped values, so spec.wandb.oidc stays unset") + require.Nil(t, dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef()) } func TestConvertTo_OIDCLegacyOidcSecret(t *testing.T) { @@ -647,8 +647,8 @@ func TestConvertTo_OIDCLegacyOidcSecret(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) - require.Equal(t, "user-oidc-secret", dst.Spec.Wandb.OIDC.ClientSecret.Name) - require.Equal(t, "MY_KEY", dst.Spec.Wandb.OIDC.ClientSecret.Key) + require.Equal(t, "user-oidc-secret", dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef().Name) + require.Equal(t, "MY_KEY", dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef().Key) raw := dst.Annotations[OIDCPendingAnnotation] var decoded map[string]interface{} @@ -673,8 +673,8 @@ func TestConvertTo_OIDCLegacyOidcSecretDefaultKey(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) - require.Equal(t, "user-oidc-secret", dst.Spec.Wandb.OIDC.ClientSecret.Name) - require.Equal(t, "OIDC_SECRET", dst.Spec.Wandb.OIDC.ClientSecret.Key) + require.Equal(t, "user-oidc-secret", dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef().Name) + require.Equal(t, "OIDC_SECRET", dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef().Key) } func TestConvertTo_OIDCValueFromRef(t *testing.T) { @@ -706,10 +706,10 @@ func TestConvertTo_OIDCValueFromRef(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) oidc := dst.Spec.Wandb.OIDC - require.Equal(t, "oidc-settings", oidc.ClientId.Name) - require.Equal(t, "clientId", oidc.ClientId.Key) - require.Equal(t, "oidc-settings", oidc.ClientSecret.Name) - require.Equal(t, "clientSecret", oidc.ClientSecret.Key) + require.Equal(t, "oidc-settings", oidc.ClientId.SecretKeyRef().Name) + require.Equal(t, "clientId", oidc.ClientId.SecretKeyRef().Key) + require.Equal(t, "oidc-settings", oidc.ClientSecret.SecretKeyRef().Name) + require.Equal(t, "clientSecret", oidc.ClientSecret.SecretKeyRef().Key) require.NotContains(t, dst.Annotations, OIDCPendingAnnotation, "no literals provided, so no annotation should be created") @@ -737,8 +737,8 @@ func TestConvertTo_OIDCMixedLiteralsAndRefs(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) - require.Equal(t, "oidc-secret", dst.Spec.Wandb.OIDC.ClientSecret.Name) - require.Empty(t, dst.Spec.Wandb.OIDC.ClientId.Name) + require.Equal(t, "oidc-secret", dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef().Name) + require.Nil(t, dst.Spec.Wandb.OIDC.ClientId.SecretKeyRef()) raw := dst.Annotations[OIDCPendingAnnotation] var decoded map[string]interface{} @@ -772,7 +772,7 @@ func TestConvertTo_OIDCValueFromWinsOverLegacyOidcSecret(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) - require.Equal(t, "valueFrom-secret", dst.Spec.Wandb.OIDC.ClientSecret.Name, + require.Equal(t, "valueFrom-secret", dst.Spec.Wandb.OIDC.ClientSecret.SecretKeyRef().Name, "secret.valueFrom should win over the legacy oidcSecret block") } @@ -807,7 +807,7 @@ func TestConvertTo_OIDCAbsent(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) require.NotContains(t, dst.Annotations, OIDCPendingAnnotation) - require.Empty(t, dst.Spec.Wandb.OIDC.ClientId.Name) + require.Nil(t, dst.Spec.Wandb.OIDC.ClientId.SecretKeyRef()) } func TestConvertTo_MySQLAllLiterals(t *testing.T) { @@ -839,8 +839,8 @@ func TestConvertTo_MySQLAllLiterals(t *testing.T) { require.NotContains(t, decoded, "passwordSecret") require.NotNil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql, "externalMysql is always allocated; reconciler fills selectors from the annotation") - require.Empty(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Host.Name) - require.Empty(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Name) + require.Nil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Host.SecretKeyRef()) + require.Nil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef()) } func TestConvertTo_MySQLLegacyPasswordSecret(t *testing.T) { @@ -861,8 +861,8 @@ func TestConvertTo_MySQLLegacyPasswordSecret(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql) - require.Equal(t, "mysql-creds", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Name) - require.Equal(t, "MYSQL_PASSWORD", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Key) + require.Equal(t, "mysql-creds", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef().Name) + require.Equal(t, "MYSQL_PASSWORD", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef().Key) raw := dst.Annotations[MySQLPendingAnnotation] var decoded map[string]interface{} @@ -886,8 +886,8 @@ func TestConvertTo_MySQLLegacyPasswordSecretDefaultKey(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql) - require.Equal(t, "mysql-creds", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Name) - require.Equal(t, "MYSQL_PASSWORD", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Key) + require.Equal(t, "mysql-creds", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef().Name) + require.Equal(t, "MYSQL_PASSWORD", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef().Key) } func TestConvertTo_MySQLValueFromRef(t *testing.T) { @@ -918,10 +918,10 @@ func TestConvertTo_MySQLValueFromRef(t *testing.T) { require.NotNil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql) conn := dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql - require.Equal(t, "mysql-settings", conn.Host.Name) - require.Equal(t, "endpoint", conn.Host.Key) - require.Equal(t, "mysql-secret", conn.Password.Name) - require.Equal(t, "password", conn.Password.Key) + require.Equal(t, "mysql-settings", conn.Host.SecretKeyRef().Name) + require.Equal(t, "endpoint", conn.Host.SecretKeyRef().Key) + require.Equal(t, "mysql-secret", conn.Password.SecretKeyRef().Name) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) require.NotContains(t, dst.Annotations, MySQLPendingAnnotation, "no literals provided, so no annotation should be created") @@ -948,8 +948,8 @@ func TestConvertTo_MySQLMixedLiteralsAndRefs(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql) - require.Equal(t, "mysql-secret", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Name) - require.Empty(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Host.Name) + require.Equal(t, "mysql-secret", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef().Name) + require.Nil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Host.SecretKeyRef()) raw := dst.Annotations[MySQLPendingAnnotation] var decoded map[string]interface{} @@ -982,7 +982,7 @@ func TestConvertTo_MySQLValueFromWinsOverPasswordSecret(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql) - require.Equal(t, "valueFrom-secret", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.Name, + require.Equal(t, "valueFrom-secret", dst.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Password.SecretKeyRef().Name, "password.valueFrom should win over the legacy passwordSecret block") } @@ -1052,7 +1052,7 @@ func TestConvertTo_RedisAllLiterals(t *testing.T) { require.NotContains(t, decoded, "secret") require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis, "externalRedis is always allocated; reconciler fills selectors from the annotation") - require.Empty(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Host.Name) + require.Nil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Host.SecretKeyRef()) } func TestConvertTo_RedisLegacySecretRef(t *testing.T) { @@ -1072,8 +1072,8 @@ func TestConvertTo_RedisLegacySecretRef(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) - require.Equal(t, "redis-creds", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.Name) - require.Equal(t, "REDIS_PASSWORD", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.Key) + require.Equal(t, "redis-creds", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.SecretKeyRef().Name) + require.Equal(t, "REDIS_PASSWORD", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.SecretKeyRef().Key) raw := dst.Annotations[RedisPendingAnnotation] var decoded map[string]interface{} @@ -1096,8 +1096,8 @@ func TestConvertTo_RedisLegacySecretRefDefaultKey(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) - require.Equal(t, "redis-creds", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.Name) - require.Equal(t, "REDIS_PASSWORD", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.Key) + require.Equal(t, "redis-creds", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.SecretKeyRef().Name) + require.Equal(t, "REDIS_PASSWORD", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.SecretKeyRef().Key) } func TestConvertTo_RedisValueFromRef(t *testing.T) { @@ -1128,10 +1128,10 @@ func TestConvertTo_RedisValueFromRef(t *testing.T) { require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) conn := dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis - require.Equal(t, "redis-settings", conn.Host.Name) - require.Equal(t, "endpoint", conn.Host.Key) - require.Equal(t, "redis-secret", conn.Password.Name) - require.Equal(t, "password", conn.Password.Key) + require.Equal(t, "redis-settings", conn.Host.SecretKeyRef().Name) + require.Equal(t, "endpoint", conn.Host.SecretKeyRef().Key) + require.Equal(t, "redis-secret", conn.Password.SecretKeyRef().Name) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) require.NotContains(t, dst.Annotations, RedisPendingAnnotation, "no literals provided, so no annotation should be created") @@ -1158,8 +1158,8 @@ func TestConvertTo_RedisMixedLiteralsAndRefs(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) - require.Equal(t, "redis-secret", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.Name) - require.Empty(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Host.Name) + require.Equal(t, "redis-secret", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.SecretKeyRef().Name) + require.Nil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Host.SecretKeyRef()) raw := dst.Annotations[RedisPendingAnnotation] var decoded map[string]interface{} @@ -1192,7 +1192,7 @@ func TestConvertTo_RedisValueFromWinsOverLegacySecret(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) - require.Equal(t, "valueFrom-secret", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.Name, + require.Equal(t, "valueFrom-secret", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Password.SecretKeyRef().Name, "password.valueFrom should win over the legacy secret block") } @@ -1236,8 +1236,8 @@ func TestConvertTo_RedisTLSValueFromInParams(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) - require.Equal(t, "redis-tls", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Name) - require.Equal(t, "enabled", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Key) + require.Equal(t, "redis-tls", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef().Name) + require.Equal(t, "enabled", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef().Key) } func TestConvertTo_RedisTLSValueFromInParameters(t *testing.T) { @@ -1260,8 +1260,8 @@ func TestConvertTo_RedisTLSValueFromInParameters(t *testing.T) { }) require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis) - require.Equal(t, "redis-tls", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Name) - require.Equal(t, "enabled", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Key) + require.Equal(t, "redis-tls", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef().Name) + require.Equal(t, "enabled", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef().Key) } func TestConvertTo_RedisTLSParamsWinsOverParameters(t *testing.T) { @@ -1293,7 +1293,7 @@ func TestConvertTo_RedisTLSParamsWinsOverParameters(t *testing.T) { }, }) require.NoError(t, src.ConvertTo(dst)) - require.Equal(t, "from-params", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Name, + require.Equal(t, "from-params", dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef().Name, "params should be checked before parameters") } @@ -1317,7 +1317,7 @@ func TestConvertTo_RedisTLSLiteralStashedInAnnotation(t *testing.T) { require.NoError(t, json.Unmarshal([]byte(raw), &decoded)) require.Equal(t, "true", decoded["tls"]) - require.Empty(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Name, + require.Nil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef(), "literal tls should not be set on the spec; reconciler materializes it") } @@ -1331,8 +1331,8 @@ func TestConvertTo_RedisTLSAbsent(t *testing.T) { }, }) require.NoError(t, src.ConvertTo(dst)) - require.Empty(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Name) - require.Empty(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.Key) + require.Nil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef()) + require.Nil(t, dst.Spec.Redis[appsv2.DefaultInstanceName].ExternalRedis.Tls.SecretKeyRef()) } // TestConvertTo_RedisTLSBooleanStashedAsString locks in that a YAML boolean @@ -1427,10 +1427,10 @@ func TestConvertTo_BucketSecretRef(t *testing.T) { require.NotNil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore) ext := dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, "bucket-creds", ext.AccessKey.Name) - require.Equal(t, "MY_ACCESS", ext.AccessKey.Key) - require.Equal(t, "bucket-creds", ext.SecretKey.Name) - require.Equal(t, "MY_SECRET", ext.SecretKey.Key) + require.Equal(t, "bucket-creds", ext.AccessKey.SecretKeyRef().Name) + require.Equal(t, "MY_ACCESS", ext.AccessKey.SecretKeyRef().Key) + require.Equal(t, "bucket-creds", ext.SecretKey.SecretKeyRef().Name) + require.Equal(t, "MY_SECRET", ext.SecretKey.SecretKeyRef().Key) require.NotContains(t, dst.Annotations, BucketPendingAnnotation, "no literals besides the secret block, so no annotation should be created") @@ -1451,8 +1451,8 @@ func TestConvertTo_BucketSecretRefDefaultKeys(t *testing.T) { require.NotNil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore) ext := dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, "ACCESS_KEY", ext.AccessKey.Key) - require.Equal(t, "SECRET_KEY", ext.SecretKey.Key) + require.Equal(t, "ACCESS_KEY", ext.AccessKey.SecretKeyRef().Key) + require.Equal(t, "SECRET_KEY", ext.SecretKey.SecretKeyRef().Key) } func TestConvertTo_BucketSecretRefEmptyName(t *testing.T) { @@ -1471,9 +1471,9 @@ func TestConvertTo_BucketSecretRefEmptyName(t *testing.T) { require.NotNil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore, "externalObjectStore is always allocated; reconciler fills selectors from the annotation") - require.Empty(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.AccessKey.Name, + require.Nil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.AccessKey.SecretKeyRef(), "empty secretName should not produce an AccessKey selector") - require.Empty(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.SecretKey.Name) + require.Nil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.SecretKey.SecretKeyRef()) raw := dst.Annotations[BucketPendingAnnotation] var decoded map[string]interface{} @@ -1498,7 +1498,7 @@ func TestConvertTo_BucketLiteralsOnlyBucket(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore, "externalObjectStore is always allocated; literals stay in the annotation") - require.Empty(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.AccessKey.Name) + require.Nil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.AccessKey.SecretKeyRef()) raw, ok := dst.Annotations[BucketPendingAnnotation] require.True(t, ok) @@ -1603,7 +1603,7 @@ func TestConvertTo_BucketSecretRefAndLiterals(t *testing.T) { require.NoError(t, src.ConvertTo(dst)) require.NotNil(t, dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore) - require.Equal(t, "bucket-creds", dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.AccessKey.Name) + require.Equal(t, "bucket-creds", dst.Spec.ObjectStore[appsv2.DefaultInstanceName].ExternalObjectStore.AccessKey.SecretKeyRef().Name) raw := dst.Annotations[BucketPendingAnnotation] var decoded map[string]interface{} @@ -1703,7 +1703,7 @@ func TestConvertRoundTrip(t *testing.T) { require.NoError(t, original.ConvertTo(firstV2)) require.Equal(t, "http://wandb.localhost", firstV2.Spec.Wandb.Hostname) require.NotNil(t, firstV2.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql) - require.Equal(t, "mysql-creds", firstV2.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Host.Name) + require.Equal(t, "mysql-creds", firstV2.Spec.MySQL[appsv2.DefaultInstanceName].ExternalMysql.Host.SecretKeyRef().Name) // Apiserver bounces through ConvertFrom internally. roundTripped := &WeightsAndBiases{} @@ -1877,10 +1877,10 @@ func TestConvertTo_ClickHouseValueFromRef(t *testing.T) { require.NotNil(t, conn) require.Nil(t, dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ManagedClickHouse, "external clickhouse must not also be managed") - require.Equal(t, "ch-settings", conn.Host.Name) - require.Equal(t, "endpoint", conn.Host.Key) - require.Equal(t, "ch-secret", conn.Password.Name) - require.Equal(t, "password", conn.Password.Key) + require.Equal(t, "ch-settings", conn.Host.SecretKeyRef().Name) + require.Equal(t, "endpoint", conn.Host.SecretKeyRef().Key) + require.Equal(t, "ch-secret", conn.Password.SecretKeyRef().Name) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) require.NotContains(t, dst.Annotations, ClickHousePendingAnnotation, "no literals provided, so no annotation should be created") @@ -1914,7 +1914,7 @@ func TestConvertTo_ClickHouseLiterals(t *testing.T) { require.NotNil(t, dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ExternalClickHouse, "externalClickhouse is always allocated; reconciler fills selectors from the annotation") - require.Empty(t, dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ExternalClickHouse.Host.Name) + require.Nil(t, dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ExternalClickHouse.Host.SecretKeyRef()) } func TestConvertTo_ClickHouseMixedLiteralsAndRefs(t *testing.T) { @@ -1939,8 +1939,8 @@ func TestConvertTo_ClickHouseMixedLiteralsAndRefs(t *testing.T) { conn := dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ExternalClickHouse require.NotNil(t, conn) - require.Equal(t, "ch-secret", conn.Password.Name) - require.Equal(t, "password", conn.Password.Key) + require.Equal(t, "ch-secret", conn.Password.SecretKeyRef().Name) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) raw := dst.Annotations[ClickHousePendingAnnotation] var decoded map[string]interface{} @@ -1968,8 +1968,8 @@ func TestConvertTo_ClickHouseLegacyPasswordSecret(t *testing.T) { conn := dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ExternalClickHouse require.NotNil(t, conn) - require.Equal(t, "ch-creds", conn.Password.Name) - require.Equal(t, "CLICKHOUSE_PASSWORD", conn.Password.Key) + require.Equal(t, "ch-creds", conn.Password.SecretKeyRef().Name) + require.Equal(t, "CLICKHOUSE_PASSWORD", conn.Password.SecretKeyRef().Key) raw := dst.Annotations[ClickHousePendingAnnotation] var decoded map[string]interface{} @@ -1993,8 +1993,8 @@ func TestConvertTo_ClickHousePasswordSecretDefaultKey(t *testing.T) { conn := dst.Spec.ClickHouse[appsv2.DefaultInstanceName].ExternalClickHouse require.NotNil(t, conn) - require.Equal(t, "ch-creds", conn.Password.Name) - require.Equal(t, "CLICKHOUSE_PASSWORD", conn.Password.Key) + require.Equal(t, "ch-creds", conn.Password.SecretKeyRef().Name) + require.Equal(t, "CLICKHOUSE_PASSWORD", conn.Password.SecretKeyRef().Key) } // TestConvertTo_ClickHousePasswordSecretMalformed: a non-string name must diff --git a/api/v2/weightsandbiases_types.go b/api/v2/weightsandbiases_types.go index 1c898285..781e2dce 100644 --- a/api/v2/weightsandbiases_types.go +++ b/api/v2/weightsandbiases_types.go @@ -180,11 +180,11 @@ type GlobalSpec struct { type ProxySpec struct { // HTTPProxy is the proxy URL for plain HTTP egress (HTTP_PROXY/http_proxy). // +optional - HTTPProxy *ProxyValue `json:"httpProxy,omitempty"` + HTTPProxy *ValueOrSecret `json:"httpProxy,omitempty"` // HTTPSProxy is the proxy URL for HTTPS egress (HTTPS_PROXY/https_proxy). // +optional - HTTPSProxy *ProxyValue `json:"httpsProxy,omitempty"` + HTTPSProxy *ValueOrSecret `json:"httpsProxy,omitempty"` // NoProxy holds EXTRA no-proxy entries appended to the operator-computed // in-cluster exclusions. Use it for external endpoints (e.g. a BYOB object @@ -194,28 +194,121 @@ type ProxySpec struct { NoProxy []string `json:"noProxy,omitempty"` } -// ProxyValue is a value-or-secret union mirroring corev1.EnvVar semantics: -// exactly one of Value or ValueFrom must be set. Credential-bearing proxy URLs -// (http://user:pass@host:port) MUST use ValueFrom; the webhook rejects userinfo -// in a literal Value so credentials never land in the CR / etcd / kubectl output. -type ProxyValue struct { - // Value is a literal proxy URL. Must not contain userinfo (credentials). +// ValueOrSecret supplies a configuration value either as a literal (Value) or +// from a Secret key (ValueFrom), mirroring corev1.EnvVar semantics: exactly one +// arm is set, enforced by the webhook. Sensitive values MUST use ValueFrom so +// they never land in the CR / etcd / kubectl output. +// +// The legacy Name/Key/Optional fields carry the historical bare-SecretKeySelector +// shape ({name, key}) so existing CRs keep validating; the defaulting webhook +// normalizes them into ValueFrom on admission. They are deprecated and will be +// removed at v2 GA. +type ValueOrSecret struct { + // Value is a literal value. // +optional Value string `json:"value,omitempty"` - // ValueFrom sources the proxy URL from a Secret key (may embed credentials). + // ValueFrom sources the value from a Secret key. // +optional - ValueFrom *ProxyValueSource `json:"valueFrom,omitempty"` + ValueFrom *SecretValueSource `json:"valueFrom,omitempty"` + + // Deprecated: use ValueFrom.secretKeyRef. Retained for backward compatibility + // with the pre-envelope {name, key} shape; normalized into ValueFrom by the + // defaulting webhook and removed at v2 GA. + // +optional + Name string `json:"name,omitempty"` + // Deprecated: use ValueFrom.secretKeyRef. + // +optional + Key string `json:"key,omitempty"` + // Deprecated: use ValueFrom.secretKeyRef. + // +optional + Optional *bool `json:"optional,omitempty"` } -// ProxyValueSource mirrors corev1.EnvVarSource (the secret case): the proxy URL -// is read from a Secret key. -type ProxyValueSource struct { +// SecretValueSource reads a value from a Secret key in the W&B namespace. +type SecretValueSource struct { // SecretKeyRef selects a key of a Secret in the W&B namespace. // +optional SecretKeyRef *corev1.SecretKeySelector `json:"secretKeyRef,omitempty"` } +// IsZero reports whether neither a literal, an envelope secret ref, nor a legacy +// secret ref is set. +func (v *ValueOrSecret) IsZero() bool { + return v == nil || (v.Value == "" && v.ValueFrom == nil && v.Name == "") +} + +// SecretKeyRef returns the effective secret selector: the canonical +// ValueFrom.SecretKeyRef when set, otherwise one synthesized from the legacy +// Name/Key/Optional fields, otherwise nil (a literal or unset value). +func (v *ValueOrSecret) SecretKeyRef() *corev1.SecretKeySelector { + if v == nil { + return nil + } + if v.ValueFrom != nil && v.ValueFrom.SecretKeyRef != nil { + return v.ValueFrom.SecretKeyRef + } + if v.Name != "" { + return &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: v.Name}, + Key: v.Key, + Optional: v.Optional, + } + } + return nil +} + +// AsEnvVar renders the value as a container EnvVar: a literal keeps the value in +// the pod spec, while a secret ref stays a live SecretKeyRef so the secret is +// never materialized. Returns a zero EnvVar (name only) when unset. +func (v *ValueOrSecret) AsEnvVar(name string) corev1.EnvVar { + if v != nil && v.Value != "" { + return corev1.EnvVar{Name: name, Value: v.Value} + } + if ref := v.SecretKeyRef(); ref != nil { + return corev1.EnvVar{Name: name, ValueFrom: &corev1.EnvVarSource{SecretKeyRef: ref.DeepCopy()}} + } + return corev1.EnvVar{Name: name} +} + +// LiteralValue wraps a literal string as a ValueOrSecret. +func LiteralValue(s string) ValueOrSecret { + return ValueOrSecret{Value: s} +} + +// Normalize rewrites the deprecated legacy {name, key} shape into the canonical +// ValueFrom.SecretKeyRef, so stored objects converge on the envelope. It is a +// no-op once the value is a literal or already an envelope secret ref. The +// defaulting webhook calls this on admission. +func (v *ValueOrSecret) Normalize() { + if v == nil || v.Name == "" || v.ValueFrom != nil { + return + } + v.ValueFrom = &SecretValueSource{SecretKeyRef: v.SecretKeyRef()} + v.Name, v.Key, v.Optional = "", "", nil +} + +// ValueFromSelector wraps an existing SecretKeySelector as the secret arm of a +// ValueOrSecret. Used by v1→v2 conversion, which classifies raw values into +// selectors before this envelope existed. +func ValueFromSelector(sel corev1.SecretKeySelector) ValueOrSecret { + return ValueOrSecret{ValueFrom: &SecretValueSource{SecretKeyRef: &sel}} +} + +// ValueFromSecret builds the canonical secret arm pointing at name/key. Used by +// status writers referencing the operator-owned connection secret. +func ValueFromSecret(name, key string, optional bool) ValueOrSecret { + return ValueOrSecret{ + ValueFrom: &SecretValueSource{ + SecretKeyRef: &corev1.SecretKeySelector{ + LocalObjectReference: corev1.LocalObjectReference{Name: name}, + Key: key, + Optional: &optional, + }, + }, + } +} + type NetworkingMode string const ( @@ -474,14 +567,25 @@ type InternalServiceAuth struct { // OidcSpec defines the structure for OpenID Connect (OIDC) configuration used in Wandb application deployments. type OidcSpec struct { - ClientId corev1.SecretKeySelector `json:"clientId,omitempty"` - ClientSecret corev1.SecretKeySelector `json:"clientSecret,omitempty"` - IssuerUrl corev1.SecretKeySelector `json:"issuerUrl,omitempty"` - AuthMethod corev1.SecretKeySelector `json:"authMethod,omitempty"` + ClientId ValueOrSecret `json:"clientId,omitempty"` + ClientSecret ValueOrSecret `json:"clientSecret,omitempty" masq:"secret"` + IssuerUrl ValueOrSecret `json:"issuerUrl,omitempty"` + AuthMethod ValueOrSecret `json:"authMethod,omitempty"` SessionLength string `json:"sessionLength,omitempty"` } +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (o *OidcSpec) Normalize() { + if o == nil { + return + } + o.ClientId.Normalize() + o.ClientSecret.Normalize() + o.IssuerUrl.Normalize() + o.AuthMethod.Normalize() +} + type ManagedInfraSpec struct { RetentionPolicy *RetentionPolicy `json:"retentionPolicy,omitempty"` @@ -509,20 +613,37 @@ type ManagedMysqlSpec struct { type MysqlConnection struct { // required - Host corev1.SecretKeySelector `json:"host,omitempty"` - Port corev1.SecretKeySelector `json:"port,omitempty"` - Database corev1.SecretKeySelector `json:"database,omitempty"` - Username corev1.SecretKeySelector `json:"username,omitempty"` - Password corev1.SecretKeySelector `json:"password,omitempty"` + Host ValueOrSecret `json:"host,omitempty"` + Port ValueOrSecret `json:"port,omitempty"` + Database ValueOrSecret `json:"database,omitempty"` + Username ValueOrSecret `json:"username,omitempty"` + Password ValueOrSecret `json:"password,omitempty" masq:"secret"` // optional - Tls corev1.SecretKeySelector `json:"tls,omitempty"` - SslCa corev1.SecretKeySelector `json:"sslCa,omitempty"` - SslCert corev1.SecretKeySelector `json:"sslCert,omitempty"` - SslKey corev1.SecretKeySelector `json:"sslKey,omitempty"` + Tls ValueOrSecret `json:"tls,omitempty"` + SslCa ValueOrSecret `json:"sslCa,omitempty"` + SslCert ValueOrSecret `json:"sslCert,omitempty"` + SslKey ValueOrSecret `json:"sslKey,omitempty" masq:"secret"` - // generated by operator - URL corev1.SecretKeySelector `json:"url,omitempty"` + // URL is the operator-assembled DSN; it embeds the password. + URL ValueOrSecret `json:"url,omitempty" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (c *MysqlConnection) Normalize() { + if c == nil { + return + } + c.Host.Normalize() + c.Port.Normalize() + c.Database.Normalize() + c.Username.Normalize() + c.Password.Normalize() + c.Tls.Normalize() + c.SslCa.Normalize() + c.SslCert.Normalize() + c.SslKey.Normalize() + c.URL.Normalize() } type MySQLConfig struct { @@ -553,13 +674,27 @@ type ManagedRedisSpec struct { } type RedisConnection struct { - Host corev1.SecretKeySelector `json:"host,omitempty"` - Port corev1.SecretKeySelector `json:"port,omitempty"` - Password corev1.SecretKeySelector `json:"password,omitempty"` - Tls corev1.SecretKeySelector `json:"tls,omitempty"` - SslCa corev1.SecretKeySelector `json:"sslCa,omitempty"` + Host ValueOrSecret `json:"host,omitempty"` + Port ValueOrSecret `json:"port,omitempty"` + Password ValueOrSecret `json:"password,omitempty" masq:"secret"` + Tls ValueOrSecret `json:"tls,omitempty"` + SslCa ValueOrSecret `json:"sslCa,omitempty"` - URL corev1.SecretKeySelector `json:"url,omitempty"` + // URL is the operator-assembled URL; it may embed the password. + URL ValueOrSecret `json:"url,omitempty" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (c *RedisConnection) Normalize() { + if c == nil { + return + } + c.Host.Normalize() + c.Port.Normalize() + c.Password.Normalize() + c.Tls.Normalize() + c.SslCa.Normalize() + c.URL.Normalize() } type RedisConfig struct { @@ -597,12 +732,25 @@ type ManagedKafkaSpec struct { } type KafkaConnection struct { - Host corev1.SecretKeySelector `json:"host,omitempty"` - Port corev1.SecretKeySelector `json:"port,omitempty"` - BrokerEndpoint corev1.SecretKeySelector `json:"brokerEndpoint,omitempty"` - ClusterID corev1.SecretKeySelector `json:"clusterID,omitempty"` + Host ValueOrSecret `json:"host,omitempty"` + Port ValueOrSecret `json:"port,omitempty"` + BrokerEndpoint ValueOrSecret `json:"brokerEndpoint,omitempty"` + ClusterID ValueOrSecret `json:"clusterID,omitempty"` - URL corev1.SecretKeySelector `json:"url,omitempty"` + // URL is the operator-assembled connection URL. + URL ValueOrSecret `json:"url,omitempty" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (c *KafkaConnection) Normalize() { + if c == nil { + return + } + c.Host.Normalize() + c.Port.Normalize() + c.BrokerEndpoint.Normalize() + c.ClusterID.Normalize() + c.URL.Normalize() } type KafkaConfig struct { @@ -654,20 +802,39 @@ const ( ) type ObjectStoreConnection struct { - // Provider selects the externalObjectStore backend (s3, gcs, or azure) from a secret key; defaults to s3 when absent. - Provider corev1.SecretKeySelector `json:"provider,omitempty"` - - Endpoint corev1.SecretKeySelector `json:"endpoint,omitempty"` - Port corev1.SecretKeySelector `json:"port,omitempty"` - AccessKey corev1.SecretKeySelector `json:"accessKey,omitempty"` - SecretKey corev1.SecretKeySelector `json:"secretKey,omitempty"` - Bucket corev1.SecretKeySelector `json:"bucket,omitempty"` + // Provider selects the externalObjectStore backend (s3, gcs, or azure); defaults to s3 when absent. + Provider ValueOrSecret `json:"provider,omitempty"` + + Endpoint ValueOrSecret `json:"endpoint,omitempty"` + Port ValueOrSecret `json:"port,omitempty"` + AccessKey ValueOrSecret `json:"accessKey,omitempty" masq:"secret"` + SecretKey ValueOrSecret `json:"secretKey,omitempty" masq:"secret"` + Bucket ValueOrSecret `json:"bucket,omitempty"` // Path is an optional key prefix within the bucket under which W&B stores its data. - Path corev1.SecretKeySelector `json:"path,omitempty"` - Region corev1.SecretKeySelector `json:"region,omitempty"` - TlsEnabled corev1.SecretKeySelector `json:"tlsEnabled,omitempty"` - ForcePathStyle corev1.SecretKeySelector `json:"forcePathStyle,omitempty"` - URL corev1.SecretKeySelector `json:"url,omitempty"` + Path ValueOrSecret `json:"path,omitempty"` + Region ValueOrSecret `json:"region,omitempty"` + TlsEnabled ValueOrSecret `json:"tlsEnabled,omitempty"` + ForcePathStyle ValueOrSecret `json:"forcePathStyle,omitempty"` + // URL is the operator-assembled connection URL; it embeds credentials as userinfo. + URL ValueOrSecret `json:"url,omitempty" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (c *ObjectStoreConnection) Normalize() { + if c == nil { + return + } + c.Provider.Normalize() + c.Endpoint.Normalize() + c.Port.Normalize() + c.AccessKey.Normalize() + c.SecretKey.Normalize() + c.Bucket.Normalize() + c.Path.Normalize() + c.Region.Normalize() + c.TlsEnabled.Normalize() + c.ForcePathStyle.Normalize() + c.URL.Normalize() } type ObjectStoreConfig struct { @@ -741,14 +908,29 @@ type ClickHouseKeeperSpec struct { } type ClickHouseConnection struct { - Host corev1.SecretKeySelector `json:"host,omitempty"` - TCPPort corev1.SecretKeySelector `json:"tcpPort,omitempty"` - HTTPPort corev1.SecretKeySelector `json:"httpPort,omitempty"` - Database corev1.SecretKeySelector `json:"database,omitempty"` - Username corev1.SecretKeySelector `json:"username,omitempty"` - Password corev1.SecretKeySelector `json:"password,omitempty"` - - URL corev1.SecretKeySelector `json:"url,omitempty"` + Host ValueOrSecret `json:"host,omitempty"` + TCPPort ValueOrSecret `json:"tcpPort,omitempty"` + HTTPPort ValueOrSecret `json:"httpPort,omitempty"` + Database ValueOrSecret `json:"database,omitempty"` + Username ValueOrSecret `json:"username,omitempty"` + Password ValueOrSecret `json:"password,omitempty" masq:"secret"` + + // URL is the operator-assembled URL; it may embed the password. + URL ValueOrSecret `json:"url,omitempty" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (c *ClickHouseConnection) Normalize() { + if c == nil { + return + } + c.Host.Normalize() + c.TCPPort.Normalize() + c.HTTPPort.Normalize() + c.Database.Normalize() + c.Username.Normalize() + c.Password.Normalize() + c.URL.Normalize() } type ClickHouseConfig struct { diff --git a/api/v2/zz_generated.deepcopy.go b/api/v2/zz_generated.deepcopy.go index 9442d0db..c541a91a 100644 --- a/api/v2/zz_generated.deepcopy.go +++ b/api/v2/zz_generated.deepcopy.go @@ -1168,12 +1168,12 @@ func (in *ProxySpec) DeepCopyInto(out *ProxySpec) { *out = *in if in.HTTPProxy != nil { in, out := &in.HTTPProxy, &out.HTTPProxy - *out = new(ProxyValue) + *out = new(ValueOrSecret) (*in).DeepCopyInto(*out) } if in.HTTPSProxy != nil { in, out := &in.HTTPSProxy, &out.HTTPSProxy - *out = new(ProxyValue) + *out = new(ValueOrSecret) (*in).DeepCopyInto(*out) } if in.NoProxy != nil { @@ -1193,46 +1193,6 @@ func (in *ProxySpec) DeepCopy() *ProxySpec { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProxyValue) DeepCopyInto(out *ProxyValue) { - *out = *in - if in.ValueFrom != nil { - in, out := &in.ValueFrom, &out.ValueFrom - *out = new(ProxyValueSource) - (*in).DeepCopyInto(*out) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxyValue. -func (in *ProxyValue) DeepCopy() *ProxyValue { - if in == nil { - return nil - } - out := new(ProxyValue) - in.DeepCopyInto(out) - return out -} - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *ProxyValueSource) DeepCopyInto(out *ProxyValueSource) { - *out = *in - if in.SecretKeyRef != nil { - in, out := &in.SecretKeyRef, &out.SecretKeyRef - *out = new(v1.SecretKeySelector) - (*in).DeepCopyInto(*out) - } -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ProxyValueSource. -func (in *ProxyValueSource) DeepCopy() *ProxyValueSource { - if in == nil { - return nil - } - out := new(ProxyValueSource) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RedisConfig) DeepCopyInto(out *RedisConfig) { *out = *in @@ -1389,6 +1349,26 @@ func (in *SecretRef) DeepCopy() *SecretRef { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *SecretValueSource) DeepCopyInto(out *SecretValueSource) { + *out = *in + if in.SecretKeyRef != nil { + in, out := &in.SecretKeyRef, &out.SecretKeyRef + *out = new(v1.SecretKeySelector) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new SecretValueSource. +func (in *SecretValueSource) DeepCopy() *SecretValueSource { + if in == nil { + return nil + } + out := new(SecretValueSource) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ServiceAccountSpec) DeepCopyInto(out *ServiceAccountSpec) { *out = *in @@ -1483,6 +1463,31 @@ func (in *TelemetryInfraStatus) DeepCopy() *TelemetryInfraStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ValueOrSecret) DeepCopyInto(out *ValueOrSecret) { + *out = *in + if in.ValueFrom != nil { + in, out := &in.ValueFrom, &out.ValueFrom + *out = new(SecretValueSource) + (*in).DeepCopyInto(*out) + } + if in.Optional != nil { + in, out := &in.Optional, &out.Optional + *out = new(bool) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ValueOrSecret. +func (in *ValueOrSecret) DeepCopy() *ValueOrSecret { + if in == nil { + return nil + } + out := new(ValueOrSecret) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WBInfraStatus) DeepCopyInto(out *WBInfraStatus) { *out = *in diff --git a/config/crd/bases/apps.wandb.com_weightsandbiases.yaml b/config/crd/bases/apps.wandb.com_weightsandbiases.yaml index 921d192e..39a497e6 100644 --- a/config/crd/bases/apps.wandb.com_weightsandbiases.yaml +++ b/config/crd/bases/apps.wandb.com_weightsandbiases.yaml @@ -530,92 +530,190 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic httpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tcpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedClickhouse: properties: @@ -1225,6 +1323,12 @@ spec: properties: httpProxy: properties: + key: + type: string + name: + type: string + optional: + type: boolean value: type: string valueFrom: @@ -1246,6 +1350,12 @@ spec: type: object httpsProxy: properties: + key: + type: string + name: + type: string + optional: + type: boolean value: type: string valueFrom: @@ -1834,131 +1944,271 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCert: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedMysql: properties: @@ -2583,144 +2833,298 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic bucket: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic - endpoint: - properties: - key: - type: string - name: - default: "" + value: type: string - optional: - type: boolean - required: - - key + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object + endpoint: + properties: + key: + type: string + name: + type: string + optional: + type: boolean + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic forcePathStyle: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic path: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic provider: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic region: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic secretKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tlsEnabled: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedObjectStore: properties: @@ -3269,79 +3673,163 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedRedis: properties: @@ -4095,53 +4583,109 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clientId: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clientSecret: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object issuerUrl: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sessionLength: type: string type: object @@ -4465,92 +5009,190 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic httpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tcpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -4714,66 +5356,136 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clusterID: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -4828,131 +5540,271 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic - password: - properties: - key: - type: string - name: - default: "" + value: type: string - optional: - type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object + password: + properties: + key: + type: string + name: + type: string + optional: + type: boolean + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCert: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -5008,144 +5860,298 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic bucket: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic endpoint: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic forcePathStyle: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic path: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic provider: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic region: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic secretKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tlsEnabled: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -5206,79 +6212,163 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean diff --git a/docs/design/wandb_v2/secret_or_value_connection_fields.md b/docs/design/wandb_v2/secret_or_value_connection_fields.md new file mode 100644 index 00000000..0fa1414e --- /dev/null +++ b/docs/design/wandb_v2/secret_or_value_connection_fields.md @@ -0,0 +1,670 @@ +# String-or-secret connection fields + +**Status:** Implemented (Option 2) — validated by `make lint`/`make test` and westest +**Scope:** `api/v2` external-connection + OIDC fields +**Target release:** during v2 beta (`2.0.0-beta.3` today), before v2 GA + +## Implementation status + +All connection types (`MysqlConnection`, `RedisConnection`, `KafkaConnection`, +`ClickHouseConnection`, `ObjectStoreConnection`) and `OidcSpec` now use the +`ValueOrSecret` envelope (Option 2), and `ProxyValue` folded onto the shared type. +Delivered and green: + +- `make build` / `make lint` (0 issues) / `make test` (exit 0). +- westest **`local-kind-ingress`** (managed SeaweedFS, status-side) and + **`local-kind-external`** (external MySQL/Redis/ClickHouse/MinIO via the legacy + `{name, key}` shape → normalized → `wandb-verify` presigned bucket round-trip) — + both pass against a locally-built operator. +- The manifest `custom-resource` env resolver is union-aware (fixes the OIDC + path); masq log-redaction is wired in `internal/logx`. + +Still open: whether to **reject** a literal on strictly-secret fields (deferred, +see [Open decisions](#open-decisions)). + +## Problem + +Every field on the external-connection structs — and on `OidcSpec` — is typed +`corev1.SecretKeySelector`, so the CRD forces the user to create a Kubernetes +Secret and a per-field `{name, key}` selector *even for values that are plainly +not secret* (hostnames, ports, database names, bucket names, regions, TLS +flags, the OIDC issuer URL, …). + +The external object-store fixture is the poster child +([external-objectstore/patch.yaml](../../../hack/testing-manifests/wandb/kustomize/overlays/external-objectstore/patch.yaml)): + +```yaml +externalObjectStore: + provider: { name: external-objectstore-connection, key: Provider } # not secret + endpoint: { name: external-objectstore-connection, key: Host } # not secret + port: { name: external-objectstore-connection, key: Port } # not secret + bucket: { name: external-objectstore-connection, key: Bucket } # not secret + region: { name: external-objectstore-connection, key: Region } # not secret + accessKey:{ name: external-objectstore-connection, key: AccessKey } # sensitive + secretKey:{ name: external-objectstore-connection, key: SecretKey } # SECRET +``` + +A user who just wants `bucket: my-bucket` and `region: us-west-2` has to stuff +those into a Secret first. We want every such field to accept **either a literal +string or a secret reference**, ideally without breaking any existing CR. + +## Goals / non-goals + +**Goals** + +- Let each connection/OIDC field be supplied as a **literal value** or a + **secret reference**. +- Apply the union type **uniformly to all fields** (per product direction), + including ones that are secret today — the union does not by itself force a + literal. +- Preserve existing CRs: an already-applied CR using the `{name, key}` shape + must keep working, ideally with **no user-visible action**. +- Converge on a single, reusable field type and consumption path (today the same + idea is expressed three different ways — see [Background](#background)). +- **Redact sensitive values from operator logs** so a secret placed in a field + never reaches the log, independent of the reject-literal policy — see + [Log redaction](#log-redaction-masq). + +**Non-goals / explicitly deferred** + +- **Whether to *reject* a literal on genuinely-secret fields** (password, + secretKey, sslKey, clientSecret) is a **follow-up decision**, tracked in + [Open decisions](#open-decisions). This doc makes every field *capable* of a + literal; the policy of forbidding one on secret fields is out of scope here. +- Managed-infra connection generation is unchanged in behavior; only the Go type + its status is written through changes. +- No change to how the operator materializes its own connection secret. + +## Background + +Three facts about the current design shape the solution. + +### 1. There is already a value-or-secret precedent in v2 + +`spec.global.proxy.httpProxy` / `httpsProxy` are `*ProxyValue` +([weightsandbiases_types.go:197-217](../../../api/v2/weightsandbiases_types.go)): + +```go +type ProxyValue struct { + Value string `json:"value,omitempty"` // literal + ValueFrom *ProxyValueSource `json:"valueFrom,omitempty"` // secret +} +type ProxyValueSource struct { + SecretKeyRef *corev1.SecretKeySelector `json:"secretKeyRef,omitempty"` +} +``` + +This mirrors `corev1.EnvVar`/`EnvVarSource`. The "exactly one of value/valueFrom" +invariant is enforced **imperatively in the validating webhook** +(`validateProxySpec`, [weightsandbiases_webhook.go:861](../../../internal/webhook/v2/weightsandbiases_webhook.go)), +not in the schema — the repo uses **zero** CEL `x-kubernetes-validations` rules +today. Consumption turns the union into an env var in `proxyValueEnvVars` +([proxy_env.go:61](../../../internal/controller/reconciler/proxy_env.go)): +literal `Value` → literal env var; `ValueFrom.SecretKeyRef` → a live +`SecretKeyRef` env source, so the credential never lands in the pod spec. + +**The connection fields lack exactly this pattern.** The design is largely +"generalize `ProxyValue` to the connection/OIDC fields." + +### 2. Consumption already collapses every field to a string + +For each external infra type, `WriteState` builds a +`map[string]corev1.SecretKeySelector` and hands it to `ResolveFields` +([common.go:114](../../../internal/controller/infra/external/common.go)), which +dereferences each selector into a `map[string]string` and writes them all into +an **operator-owned connection secret** (`wandb-mysql-connection`, etc.). Apps +consume from *that* secret, never from the user's original +(e.g. [mysql.go:42](../../../internal/controller/infra/external/mysql/mysql.go), +[objectstore.go:43](../../../internal/controller/infra/external/objectstore/objectstore.go)). + +Consequence: a **literal value slots straight into the `map[string]string` +with no secret read**. The resolution layer is the single chokepoint. + +There are actually **three** places that decode "field → string or secret", and +they should converge on one helper: + +| Path | Where | Today | +|------|-------|-------| +| External infra resolve | `ResolveFields` / `ResolveSecretKey` ([common.go:16](../../../internal/controller/infra/external/common.go)) | reads `SecretKeySelector` only | +| Manifest `custom-resource` env | `resolveCRFieldSecretSelector` + `resolveCRFieldEnvValue` ([reconcile_v2.go:1332-1367](../../../internal/controller/reconciler/reconcile_v2.go)) | tries `SecretKeySelector`, else literal scalar | +| Proxy env | `proxyValueEnvVars` ([proxy_env.go:61](../../../internal/controller/reconciler/proxy_env.go)) | already union-aware | + +The **manifest `custom-resource` path is a live breakage risk**: the server +manifest can source an env var from a dotted CR path (e.g. an OIDC field). Today +`resolveCRFieldSecretSelector` unmarshals the terminal node as +`{name, key}`. If that node becomes a `{value, valueFrom}` union, **both** +resolvers fail to match it and the env var silently resolves to nothing. This +path must be made union-aware as part of the change. + +### 3. The connection structs are dual-purpose (spec **and** status) + +`MysqlConnection`, `RedisConnection`, `ObjectStoreConnection`, and +`ClickHouseConnection` are used **both** as user spec +(`spec.[].external*`) **and** as operator-written status +(`status.Status[].connection`, [weightsandbiases_types.go:840-863](../../../api/v2/weightsandbiases_types.go)). +`KafkaConnection` is **status-only** (Kafka is managed-only; there is no +`externalKafka`). Every `URL` field is operator-generated output. + +Status writers construct these structs with `SecretKeySelector` literals +pointing at the operator connection secret — in both the external readers +([mysql.go:108](../../../internal/controller/infra/external/mysql/mysql.go)) +and the managed writers (`moco/conn.go:95`, `opstree/conn.go:97`, +`bufstream/conn.go:93`, `altinity/conn.go:108`, `objectstore/secret.go:64`). +So **changing the struct type ripples into every status writer**, not just spec +input. This is fine — status always uses the *secret* arm of the union — but it +must be handled (see [Consumption changes](#consumption-changes)). + +## The core constraint: no scalar-or-object in a structural schema + +The nicest UX would be one field that accepts **either** a bare string **or** an +object on the same path: + +```yaml +bucket: my-bucket # string +bucket: { name: s, key: Bucket } # object +``` + +This is **not expressible in a structural CRD schema**. Structural schemas +require a single `type` per node. The only polymorphism escape hatches are: + +- `x-kubernetes-int-or-string` — int-vs-string scalar only; cannot model + scalar-vs-object. +- `x-kubernetes-preserve-unknown-fields` — allows anything but **disables + validation and pruning** for that subtree. This CRD is served at v1+v2 with a + conversion webhook and must stay structural; going schemaless on a typed field + is a regression (it is confined today to the legacy v1 `spec.values` blob). + +Therefore the field must be a **wrapper object** with mutually-exclusive +sub-fields — the `value` / `valueFrom` envelope, exactly like `ProxyValue`. +This is what forces the backward-compat discussion below: today the field *is* +the bare `{name, key}` object, and a wrapper changes that shape. + +### Pruning happens before webhooks (why this matters) + +For CRDs, unknown fields are **pruned at decode time, before mutating admission +webhooks run**. So a field that is not in the schema cannot be recovered by a +webhook. Any scheme that wants a webhook to *migrate* the legacy `{name, key}` +shape must **keep `name`/`key` in the schema** so they survive pruning long +enough for the webhook to move them. This is the linchpin of Option 2 below. + +## Proposed type + +Introduce one shared type (generalizing `ProxyValueSource`): + +```go +// ValueOrSecret supplies a configuration value either as a literal or from a +// Secret key. Exactly one of Value or ValueFrom is set; the webhook enforces it. +type ValueOrSecret struct { + // Value is a literal value. + // +optional + Value string `json:"value,omitempty"` + + // ValueFrom sources the value from a Secret key. + // +optional + ValueFrom *SecretValueSource `json:"valueFrom,omitempty"` +} + +// SecretValueSource reads a value from a Secret key in the W&B namespace. +type SecretValueSource struct { + // +optional + SecretKeyRef *corev1.SecretKeySelector `json:"secretKeyRef,omitempty"` +} +``` + +**`ProxyValue` folds onto this shared type** (decided). `ProxyValue` / +`ProxyValueSource` are removed; `SecretValueSource` replaces `ProxyValueSource`; +`ProxySpec.HTTPProxy` / `HTTPSProxy` become `*ValueOrSecret`. Proxy already uses +the `value`/`valueFrom` envelope, so existing proxy CRs are unaffected; its +URL-specific checks (http/https scheme, userinfo rejection) stay in the webhook +as a layer on top of the shared exactly-one-of validator (see +[Validation](#validation)). Proxy fields inherit the deprecated legacy +`name`/`key` fields too — vestigial for proxy, removed at GA with everyone else. + +Connection/OIDC fields change from `corev1.SecretKeySelector` to `ValueOrSecret`, +and `status.Status[].connection` uses the **same** `ValueOrSecret` object +as the spec (decided) — status always populates the secret arm. The definitive +shipped shape carries the deprecated legacy selector fields for backward +compatibility; see [Option 2](#option-2--valuevaluefrom-envelope--auto-normalizing-defaulter-recommended). + +Shared consumption + construction helpers (one implementation, used by all three +paths in the table above): + +```go +// SecretKeyRef returns the effective selector (ValueFrom.SecretKeyRef, else the +// legacy name/key), or nil for a literal/unset value. +func (v *ValueOrSecret) SecretKeyRef() *corev1.SecretKeySelector + +// IsZero reports "neither literal nor secret ref set". +func (v *ValueOrSecret) IsZero() bool + +// AsEnvVar produces a literal or SecretKeyRef-backed EnvVar (à la proxyValueEnvVars). +func (v *ValueOrSecret) AsEnvVar(name string) corev1.EnvVar + +// Normalize rewrites the legacy {name,key} shape into ValueFrom. Each connection +// type + OidcSpec has its own Normalize() that calls this on every field; the +// defaulter invokes them (see Log-redaction / Validation). +func (v *ValueOrSecret) Normalize() + +// LiteralValue / ValueFromSecret / ValueFromSelector are the constructors. +func LiteralValue(s string) ValueOrSecret +func ValueFromSecret(name, key string, optional bool) ValueOrSecret // status writers +func ValueFromSelector(sel corev1.SecretKeySelector) ValueOrSecret // v1→v2 conversion + +// Resolution (needs a client) lives in internal/controller/infra/external: +// ResolveValue / ResolveValueFields; and utils.ConnSecretResolver.ValueOrSecret. +``` + +> **Naming note (implementation):** the status-writer constructor is +> `ValueFromSecret`, **not** `SecretRef` — `SecretRef` collides with the existing +> `type SecretRef struct` used by `ListenerTLSConfig.CertificateRef`. +> +> **Deprecation-lint note:** all access to the deprecated legacy `name`/`key` +> fields lives in `api/v2` methods (`SecretKeyRef`, `Normalize`). staticcheck +> `SA1019` exempts use within the declaring package, so consumers never touch the +> deprecated fields directly. + +Secret-bearing fields on the connection/OIDC structs additionally carry a +`masq:"secret"` struct tag for log redaction — see +[Log redaction](#log-redaction-masq). + +## Backward-compatibility options (the key decision) + +The requirement is "existing CRs keep working, ideally with no user-visible +action." Two viable options; they differ in **API cleanliness now vs. lifecycle +cost**. A third (clean break) is listed as considered-and-rejected given the +no-break requirement. + +### Option 1 — Flat superset (permanent), lowest cost + +Make the type a superset that keeps the historical selector fields at the top +level and *adds* `value`: + +```go +type ValueOrSecret struct { + Value string `json:"value,omitempty"` // literal (new) + Name string `json:"name,omitempty"` // secret ref (legacy shape, kept) + Key string `json:"key,omitempty"` + Optional *bool `json:"optional,omitempty"` +} +``` + +- Existing `host: {name, key}` **stays valid and populated** — zero migration, + zero risk, nothing rewrites the user's object. +- New users write `host: {value: "db.example"}`. +- **Downside:** permanently diverges from the `value`/`valueFrom` idiom that + `ProxyValue` established in the *same* CRD; `name`/`key` sitting alongside + `value` is a little ad-hoc, and there is no `valueFrom` grouping. + +Complexity: **low.** New type + deepcopy, union-aware resolve helpers, mechanical +constructor swaps in status writers and conversion, and per-field validation. No +mutating normalization, no deprecation lifecycle. + +### Option 2 — `value`/`valueFrom` envelope + auto-normalizing defaulter (recommended) + +Adopt the idiomatic envelope as **canonical**, but during the beta bridge keep +the legacy selector fields in the schema so they survive pruning, and have the +**mutating defaulter normalize them into `valueFrom` on admission**: + +```go +type ValueOrSecret struct { + Value string `json:"value,omitempty"` + ValueFrom *SecretValueSource `json:"valueFrom,omitempty"` + + // Deprecated: legacy inline secret-ref shape, retained for backward compat + // through v2 beta. The defaulter rewrites these into ValueFrom.SecretKeyRef. + // Removed at v2 GA. + Name string `json:"name,omitempty"` + Key string `json:"key,omitempty"` + Optional *bool `json:"optional,omitempty"` +} +``` + +Normalization slots into the existing `WeightsAndBiasesCustomDefaulter.Default` +alongside `applyMySQLDefaults` etc. +([weightsandbiases_webhook.go:71](../../../internal/webhook/v2/weightsandbiases_webhook.go)): +for every connection/OIDC field, if `Name != ""` and `ValueFrom == nil`, set +`ValueFrom.SecretKeyRef = {Name, Key, Optional}` and clear the legacy fields. + +- Existing `host: {name, key}` keeps working; on next apply the stored object is + silently rewritten to `host: {valueFrom: {secretKeyRef: {name, key}}}` — the + **user never takes an action**, matching the "automate it" ask. +- New users write the clean `value` / `valueFrom` shape. +- At **v2 GA**, drop `name`/`key`/`optional` from the type; everything stored has + been normalized by then, and the API is fully idiomatic. +- The normalization logic mirrors the existing v1→v2 `classifyValueFromOrLiteral` + helper ([weightsandbiases_conversion_mapping.go:784](../../../api/v1/weightsandbiases_conversion_mapping.go)), + so the pattern is not new to the codebase. + +Complexity: **Option 1 + a normalizing defaulter + a GA-removal task.** Schema is +a strict superset of Option 1 during the bridge (both carry `value` + legacy +`name`/`key`); Option 2 additionally carries `valueFrom` and the normalizer. + +### Option 3 — Clean break (envelope only) — rejected + +Envelope only, no legacy fields, document manual migration. Cleanest type and +least code, but an existing beta CR with `host: {name, key}` would be **pruned to +`host: {}` on upgrade — silent data loss**. Rejected given the no-break +requirement; listed for completeness (viable only if we accept editing every +existing v2 CR). + +### Recommendation + +**Decided: Option 2.** Rationale: v2 is still beta (`2.0.0-beta.3`), which is the cheap +moment to establish the clean, `ProxyValue`-consistent shape and schedule legacy +removal at GA; the normalizing defaulter delivers the "user never sees it" +guarantee; and doing it now avoids a *second* API migration later. If minimizing +scope is paramount and the team is comfortable with a permanently-flat union, +**Option 1 delivers the identical user-facing capability for materially less +work** — it is a legitimate fallback, not a wrong answer. + +| | Option 1 (flat) | Option 2 (envelope + normalize) | Option 3 (clean break) | +|---|---|---|---| +| Existing CRs keep working | ✅ verbatim | ✅ auto-normalized | ❌ pruned/broken | +| User action required | none | none | edit every CR | +| End-state API shape | flat, non-idiomatic | idiomatic, matches ProxyValue | idiomatic | +| Mutating webhook work | none | normalizer (mechanical) | none | +| Lifecycle cost | none | deprecate + remove at GA | none | +| Second migration later? | maybe (if we later want envelope) | no | no | + +## Field inventory & classification + +From a repo-wide audit of `corev1.SecretKeySelector` in `api/v2`. All live in +[weightsandbiases_types.go](../../../api/v2/weightsandbiases_types.go); +`application_types.go` has none. + +| Struct | Field(s) | Class | Notes | +|---|---|---|---| +| `MysqlConnection` | host, port, database, tls, sslCa, sslCert | non-secret | | +| | username | arguable | credential-ish, often not secret | +| | password, sslKey | **secret** | | +| | url | output | operator-generated DSN | +| `RedisConnection` | host, port, tls, sslCa | non-secret | | +| | password | **secret** | | +| | url | output | | +| `ClickHouseConnection` | host, tcpPort, httpPort, database | non-secret | | +| | username | arguable | | +| | password | **secret** | | +| | url | output | | +| `ObjectStoreConnection` | provider, endpoint, port, bucket, path, region, tlsEnabled, forcePathStyle | non-secret | | +| | accessKey | arguable | key *id* | +| | secretKey | **secret** | | +| | url | output | | +| `KafkaConnection` | host, port, brokerEndpoint, clusterID, url | output | status-only (managed-only), no `externalKafka` | +| `OidcSpec` | issuerUrl, authMethod | non-secret | | +| | clientId | arguable | | +| | clientSecret | **secret** | | + +Per product direction, **all** of these become `ValueOrSecret`. `url` and the +`KafkaConnection` fields are operator-generated output; they simply never use the +literal arm. The genuinely-secret set (password, sslKey, secretKey, +clientSecret) is what the deferred "reject literal" policy would target. + +## Consumption changes (as implemented) + +1. **External resolve** — added `ResolveValue` / `ResolveValueFields` + ([common.go](../../../internal/controller/infra/external/common.go)) alongside + the (now unused for connections) `ResolveFields`; a literal is used as-is, a + secret arm is read via `ResolveSecretKey`. All external `WriteState`s + (mysql/redis/clickhouse/objectstore) feed `ResolveValueFields`. The + objectstore read path (`utils.ConnSecretResolver`) gained a `ValueOrSecret` + method. + +2. **Manifest `custom-resource` resolver** (done) — + `resolveCRFieldSecretSelector` / `resolveCRFieldEnvValue` + ([reconcile_v2.go](../../../internal/controller/reconciler/reconcile_v2.go)) + are union-aware: the terminal CR node is unmarshalled into `ValueOrSecret`; + `SecretKeyRef()` → a `SecretKeyRef` env source, a literal → an env value. This + fixed the OIDC breakage noted in [Background](#background). + +3. **Status writers** — external `ReadState` and managed writers + (`moco/conn.go`, `opstree/conn.go`, `bufstream/conn.go`, `altinity/conn.go`, + `objectstore/secret.go`) construct the connection struct via + `apiv2.ValueFromSecret(name, key, optional)` instead of `SecretKeySelector{…}`. + The internal `objectstore.ConnInfo.*Ref` fields stay `corev1.SecretKeySelector` + (status is always secret-backed), consumed by ClickHouse/Bufstream. + +4. **Other consumers updated to `SecretKeyRef()`**: `custom_ca.go` (external + MySQL/Redis TLS CA volume mounts + checksum), `kafka.go` (bootstrap host from + the status connection), and every `pods.go` env-source case + (mysql/redis/clickhouse/kafka/bucket). + Mechanical. + +4. **Env construction** — `proxyValueEnvVars` + ([proxy_env.go:61](../../../internal/controller/reconciler/proxy_env.go)) is + retyped to `*ValueOrSecret` (or replaced by the shared `AsEnvVar`), and OIDC / + any directly-surfaced connection field uses the same helper — preserving the + "secret stays a `SecretKeyRef`, never a literal in the pod spec" property. + +## Validation + +Follow the `ProxyValue` precedent — **imperative Go in the validating webhook**, +consistent with the repo (no CEL today): + +- Generalize a `validateValueOrSecret(v, path)` helper enforcing **exactly one of + `value` / `valueFrom`** (and, during the Option 2 bridge, treating legacy + `name`/`key` as the secret arm). Replaces the selector-only + `validateRequiredSecretSelector` + ([weightsandbiases_webhook.go:552](../../../internal/webhook/v2/weightsandbiases_webhook.go)) + and is called from each per-infra validator (`validateMySQLSpec`, + `validateObjectStoreSpec`, …). `validateProxySpec` reuses the same helper for + the exactly-one-of check and keeps its URL scheme / userinfo checks as + proxy-specific additions on top. +- Keep the credential-redaction discipline: when rejecting a literal that might + contain a secret, pass a constant `"[redacted]"` as the `field.Invalid` value + (as `validateProxySpec` does), never the offending string. +- **Deferred:** whether to reject a literal on the secret-classified fields (see + [Open decisions](#open-decisions)). If adopted, it is one extra check in the + same helper, keyed by a per-field "isSecret" flag. +- **CEL: skipped for now** (decided). A single CEL rule + `has(self.value) != has(self.valueFrom)` per field would enforce the exclusion + even when the webhook is bypassed (GitOps dry-run), and remains available + (k8s 1.35, controller-gen v0.19.0) as a later defense-in-depth add — but it + would be the repo's first CEL rule, and the Go webhook is authoritative + regardless. Not in scope for this change. + +## Log redaction (masq) + +**Why now.** The union lets a user place a real secret in a field's `value`, and +until the [deferred reject-literal policy](#open-decisions) forbids that, the +operator can log connection data during reconcile. Concretely, the manifest +`custom-resource` resolver already logs a *resolved literal* value at debug level +(`logger.Debug("field found in CR", …, "value", val)`, +[pods.go:418](../../../internal/controller/reconciler/pods.go)), and the +resolved-fields path (see below) holds plaintext credentials in memory. We want +defense-in-depth: a sensitive value should never reach the operator log even if +it reaches the CR. + +**Library.** [`github.com/m-mizutani/masq`](https://github.com/m-mizutani/masq) +(Apache-2.0 — matches the operator; min Go 1.24, fine on the repo's 1.26). +`masq.New(opts...)` returns a `ReplaceAttr` function for `slog.HandlerOptions`. +The operator **already logs through `slog`** ([main.go:165](../../../cmd/manager/main.go), +[internal/logx](../../../internal/logx/handler.go)), so masq drops in with no +change to the logging stack. + +**Wiring — one central place, composed with the existing `ReplaceAttr`.** +`internal/logx` owns handler construction ([handler.go:10](../../../internal/logx/handler.go)): +JSON/Text handlers read `ReplaceAttr` off `opts.HandlerOptions` (unset today), +while the Pretty/tint handler sets its **own** `ReplaceAttr` +([pretty.go:14](../../../internal/logx/pretty.go)). There is only one +`ReplaceAttr` slot per handler, so masq must be **composed**, not assigned +blindly: + +```go +// internal/logx — build once, apply to every format. +var redact = masq.New(masq.WithTag("secret")) // default tag key is "masq" + +func chainReplaceAttr(fns ...func([]string, slog.Attr) slog.Attr) func([]string, slog.Attr) slog.Attr { + return func(groups []string, a slog.Attr) slog.Attr { + for _, fn := range fns { + if fn != nil { + a = fn(groups, a) + } + } + return a + } +} +``` + +- **JSON/Text:** in `withDefaults`/`NewHandler`, set + `opts.HandlerOptions.ReplaceAttr = chainReplaceAttr(opts.HandlerOptions.ReplaceAttr, redact)`. +- **Pretty:** `BuildPrettyHandler` composes `redact` with its existing LoggerKey + rename (they touch different attrs, so order is safe; run masq last so + redaction is final). + +Centralizing in `logx` covers every logger — controller-runtime via +`NewLogrLogger` and the direct `NewSlogLogger`. + +**Tagging.** With `masq.WithTag("secret")`, any struct field tagged +`masq:"secret"` is replaced wholesale with masq's redaction placeholder when that +struct is logged as an attribute: + +```go +type MysqlConnection struct { + // ... + Password ValueOrSecret `json:"password,omitempty" masq:"secret"` + SslKey ValueOrSecret `json:"sslKey,omitempty" masq:"secret"` + URL ValueOrSecret `json:"url,omitempty" masq:"secret"` // assembled DSN embeds the password +} +``` + +Fields to tag `masq:"secret"`: + +| Struct | Fields | +|---|---| +| `MysqlConnection` | password, sslKey, url | +| `RedisConnection` | password, url | +| `ClickHouseConnection` | password, url | +| `ObjectStoreConnection` | secretKey, accessKey, url | +| `OidcSpec` | clientSecret | +| `KafkaConnection` | url | + +`accessKey` and every `url` are tagged because the assembled S3/GCS/Azure and DSN +URLs embed credentials as userinfo (see the URL builders in +[objectstore.go](../../../internal/controller/infra/external/objectstore/objectstore.go) +and [mysql.go](../../../internal/controller/infra/external/mysql/mysql.go)). The +`masq` tag is inert to controller-gen and deepcopy-gen — **no CRD/schema impact**. + +**Honest limitation — tagging is necessary but not sufficient.** Tag-based +masking fires only when a value is logged **as part of the tagged struct**. Two +leak paths it does *not* cover: + +1. The resolved `map[string]string` in `ResolveFields`/`WriteState` + ([common.go:114](../../../internal/controller/infra/external/common.go), + [mysql.go:88](../../../internal/controller/infra/external/mysql/mysql.go)) + holds the plaintext password and the full DSN — a bare map, not a tagged + struct. +2. Ad-hoc string logging like the `pods.go:418` debug line above (the literal + arm of the `custom-resource` resolver). + +Mitigations, most robust first: + +- **Discipline:** never log the resolved connection map or a resolved value; + audit and remove/guard existing sites (the `pods.go:418` debug line is the + known offender). +- **Typed secret + `WithType`:** give resolved secret strings a dedicated type + (`type Secret string`) and add `masq.WithType[Secret]()`, so a secret is masked + wherever it is logged, independent of struct context. Strongest option for the + resolved-values path; recommended alongside tagging. +- **Coarse censors:** `masq.WithFieldName("password"|"secretKey"|…)` / + `masq.WithContain(...)` as belt-and-suspenders. Lower precision; optional. + +**Residual risk:** a secret typed into a *non-secret* field's `value` is not +masked (that field isn't tagged). Closing that residual is exactly the deferred +reject-literal policy; masking the known-secret fields is the interim mitigation. + +## Conversion & legacy-migration impact + +`ConvertFrom` (v2→v1) round-trips through annotations and never reads these Go +types — **unaffected**. The v1→v2 direction and the reconciler's legacy +migration construct `SecretKeySelector` literals and must be updated to build the +`ValueOrSecret` secret arm instead: + +- `api/v1/weightsandbiases_conversion_mapping.go` — the `setRef` field-tables + (`mysqlFields`, `redisFields`, `clickHouseFields`, `oidcFields`), the + `mapBucket` access/secret-key literals, and the legacy password/clientSecret + ref blocks. `classifyValueFromOrLiteral` + ([:784](../../../api/v1/weightsandbiases_conversion_mapping.go)) already + distinguishes literal vs secret — it maps cleanly onto `ValueOrSecret` + (literal → `Value`; valueFrom → the secret arm), arguably *simplifying* this + code. +- `internal/controller/reconciler/migrate_legacy.go` — the `fill` closures now + target `*apiv2.ValueOrSecret`, guard on `!target.IsZero()`, and assign + `apiv2.ValueFromSecret(secretName, dataKey, false)`. The old `secretSelector` + helper was removed. + +Done as implemented; the compiler enumerated every site. `ConvertFrom` (annotation +round-trip) was untouched, as predicted. + +## Worked example (object store) + +Before (today, all-secret-ref): + +```yaml +externalObjectStore: + endpoint: { name: os-conn, key: Host } + bucket: { name: os-conn, key: Bucket } + region: { name: os-conn, key: Region } + accessKey:{ name: os-conn, key: AccessKey } + secretKey:{ name: os-conn, key: SecretKey } +``` + +After (Option 2 — literals for non-secrets, secret arm for credentials): + +```yaml +externalObjectStore: + endpoint: { value: s3.us-west-2.amazonaws.com } + bucket: { value: my-wandb-bucket } + region: { value: us-west-2 } + accessKey:{ valueFrom: { secretKeyRef: { name: os-conn, key: AccessKey } } } + secretKey:{ valueFrom: { secretKeyRef: { name: os-conn, key: SecretKey } } } +``` + +The existing all-`{name, key}` form continues to apply unchanged; under Option 2 +the defaulter rewrites each entry to the `valueFrom` form on next admission. + +## Rollout (completed) + +1. ✅ Added `ValueOrSecret` / `SecretValueSource` + helpers; switched all + connection/OIDC fields to the Option 2 envelope (deprecated legacy + `name`/`key`/`optional` retained through beta). Folded + `ProxyValue`/`ProxyValueSource` onto `ValueOrSecret`/`SecretValueSource` + (retyped `ProxySpec.HTTPProxy`/`HTTPSProxy`; updated `proxy_env.go`). Added + `masq:"secret"` tags to the secret-bearing fields (password/sslKey/secretKey/ + clientSecret + credential-bearing URLs). +2. ✅ `make manifests generate sync-crd-embed`. +3. ✅ Updated external resolve, the `custom-resource` resolver, status writers, + conversion (`mapMySQL/Redis/ClickHouse/OIDC/Bucket`), `migrate_legacy`, + `custom_ca.go`, `kafka.go`, `pods.go`, and per-type webhook validation. +4. ✅ Added the normalizing defaulter (`normalizeConnections`, all external + conns + OIDC); deprecation markers on the legacy fields. **Follow-up: drop + legacy `name`/`key`/`optional` at v2 GA.** +5. ✅ Wired masq into `internal/logx` (`masq.New(masq.WithTag("secret"))` composed + via a `chainReplaceAttr` helper into the JSON/Text `HandlerOptions` and the + Pretty/tint handler); added `redact_test.go`. +6. ⏳ Fixture refresh under + [hack/testing-manifests/](../../../hack/testing-manifests/) to showcase + literals is optional (the defaulter normalizes the existing all-secret-ref + fixtures); westest `local-kind-external` already exercises the legacy shape. +7. ✅ `make lint` (0 issues) && `make test` (exit 0); westest `local-kind-ingress` + + `local-kind-external` green. + +## Open decisions + +1. **Backward-compat approach:** ✅ **Decided — Option 2** (envelope + + normalizing defaulter + deprecate legacy at GA). +2. **Reject literals on secret fields?** ⏳ **Still open** (deferred, per + product) — allow everywhere / reject on {password, sslKey, secretKey, + clientSecret} / warn. Log redaction is the interim mitigation until this + lands, and the typed-`Secret` question (below) rides along with it. +3. **Log redaction — typed `Secret` + `masq.WithType`?** ✅ **Decided — not + now.** Rely on struct-field tags + logging discipline; revisit together with + decision #2. +4. **Mask the *arguable* fields (`username`/`clientId`)?** ✅ **Decided — no.** + Only the strictly-secret set is tagged, plus `accessKey` and every `url` + (URLs embed credentials as userinfo). `username`/`clientId` stay untagged. +5. **Fold `ProxyValue` onto the shared `ValueOrSecret`?** ✅ **Decided — yes**, + as part of this change. +6. **CEL exclusion rule?** ✅ **Decided — no** (Go-only for now; CEL noted as a + possible later add). +7. **Type used for status?** ✅ **Decided — shared `ValueOrSecret`**, same + object as the spec (status always uses the secret arm). diff --git a/go.mod b/go.mod index 2db2647c..305d7c48 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/imdario/mergo v0.3.16 github.com/kedacore/keda/v2 v2.18.3 github.com/lmittmann/tint v1.1.2 + github.com/m-mizutani/masq v0.2.2 github.com/maxbrunsfeld/counterfeiter/v6 v6.12.2 github.com/nginx/nginx-gateway-fabric v1.6.2 github.com/onsi/ginkgo/v2 v2.28.1 diff --git a/go.sum b/go.sum index 78200140..d04b3ea7 100644 --- a/go.sum +++ b/go.sum @@ -316,6 +316,10 @@ github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhn github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE= github.com/lmittmann/tint v1.1.2 h1:2CQzrL6rslrsyjqLDwD11bZ5OpLBPU+g3G/r5LSfS8w= github.com/lmittmann/tint v1.1.2/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE= +github.com/m-mizutani/gt v0.1.0 h1:HCz+dE6zmlpceb4smal4hOyZyS+WHxGtiOTUJwA9n1A= +github.com/m-mizutani/gt v0.1.0/go.mod h1:0MPYSfGBLmYjTduzADVmIqD58ELQ5IfBFiK/f0FmB3k= +github.com/m-mizutani/masq v0.2.2 h1:C049XdUabx1T+cUuWUiWg9gGo7onrMAgqRe71tyBi58= +github.com/m-mizutani/masq v0.2.2/go.mod h1:tDXVSkv0TlxdxV8dfkmKj974VQozK9llZSSCHhEkcJE= github.com/maruel/natural v1.1.1 h1:Hja7XhhmvEFhcByqDoHz9QZbkWey+COd9xWfCfn1ioo= github.com/maruel/natural v1.1.1/go.mod h1:v+Rfd79xlw1AgVBjbO0BEQmptqb5HvL/k9GRHB7ZKEg= github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= diff --git a/hack/tilt/wandbcr/main.go b/hack/tilt/wandbcr/main.go index d7841600..ed3414cc 100644 --- a/hack/tilt/wandbcr/main.go +++ b/hack/tilt/wandbcr/main.go @@ -472,38 +472,38 @@ func patchTelemetry(cr *v2.WeightsAndBiases, observabilityMode string) error { func patchExternalInfra(cr *v2.WeightsAndBiases, opts Options) { if opts.ExternalMySQL { conn := &v2.MysqlConnection{ - Host: secretKeySelector(externalMySQLSecret, "Host"), - Port: secretKeySelector(externalMySQLSecret, "Port"), - Database: secretKeySelector(externalMySQLSecret, "Database"), - Username: secretKeySelector(externalMySQLSecret, "Username"), - Password: secretKeySelector(externalMySQLSecret, "Password"), + Host: valueFromSecret(externalMySQLSecret, "Host"), + Port: valueFromSecret(externalMySQLSecret, "Port"), + Database: valueFromSecret(externalMySQLSecret, "Database"), + Username: valueFromSecret(externalMySQLSecret, "Username"), + Password: valueFromSecret(externalMySQLSecret, "Password"), } if opts.CustomCA { - conn.SslCa = secretKeySelector(externalMySQLTLSSecret, "ca.crt") + conn.SslCa = valueFromSecret(externalMySQLTLSSecret, "ca.crt") } cr.Spec.MySQL[v2.DefaultInstanceName] = v2.MySQLSpec{ExternalMysql: conn} } if opts.ExternalRedis { conn := &v2.RedisConnection{ - Host: secretKeySelector(externalRedisSecret, "Host"), - Port: secretKeySelector(externalRedisSecret, "Port"), + Host: valueFromSecret(externalRedisSecret, "Host"), + Port: valueFromSecret(externalRedisSecret, "Port"), } if opts.CustomCA { - conn.SslCa = secretKeySelector(externalRedisTLSSecret, "ca.crt") + conn.SslCa = valueFromSecret(externalRedisTLSSecret, "ca.crt") } cr.Spec.Redis[v2.DefaultInstanceName] = v2.RedisSpec{ExternalRedis: conn} } if opts.ExternalObjectStore { cr.Spec.ObjectStore[v2.DefaultInstanceName] = v2.ObjectStoreSpec{ExternalObjectStore: &v2.ObjectStoreConnection{ - Provider: secretKeySelector(externalObjectStoreSecret, "Provider"), - Endpoint: secretKeySelector(externalObjectStoreSecret, "Host"), - Port: secretKeySelector(externalObjectStoreSecret, "Port"), - Bucket: secretKeySelector(externalObjectStoreSecret, "Bucket"), - Region: secretKeySelector(externalObjectStoreSecret, "Region"), - AccessKey: secretKeySelector(externalObjectStoreSecret, "AccessKey"), - SecretKey: secretKeySelector(externalObjectStoreSecret, "SecretKey"), + Provider: valueFromSecret(externalObjectStoreSecret, "Provider"), + Endpoint: valueFromSecret(externalObjectStoreSecret, "Host"), + Port: valueFromSecret(externalObjectStoreSecret, "Port"), + Bucket: valueFromSecret(externalObjectStoreSecret, "Bucket"), + Region: valueFromSecret(externalObjectStoreSecret, "Region"), + AccessKey: valueFromSecret(externalObjectStoreSecret, "AccessKey"), + SecretKey: valueFromSecret(externalObjectStoreSecret, "SecretKey"), }} } } @@ -606,9 +606,8 @@ func stringPtr(value string) *string { return &value } -func secretKeySelector(secretName, key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: secretName}, - Key: key, - } +// valueFromSecret wraps a secret key as a ValueOrSecret (the envelope shape used +// by the object-store connection fields). +func valueFromSecret(secretName, key string) v2.ValueOrSecret { + return v2.ValueFromSecret(secretName, key, false) } diff --git a/hack/tilt/wandbcr/main_test.go b/hack/tilt/wandbcr/main_test.go index 9787400f..83aa66c0 100644 --- a/hack/tilt/wandbcr/main_test.go +++ b/hack/tilt/wandbcr/main_test.go @@ -89,12 +89,12 @@ func TestBuildCRExternalMySQLOnly(t *testing.T) { if cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql == nil { t.Fatalf("external mysql should be configured") } - assertSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Host, externalMySQLSecret, "Host") - assertSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Port, externalMySQLSecret, "Port") - assertSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Database, externalMySQLSecret, "Database") - assertSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Username, externalMySQLSecret, "Username") - assertSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Password, externalMySQLSecret, "Password") - assertEmptySelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.SslCa, "mysql sslCa") + assertValueSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Host, externalMySQLSecret, "Host") + assertValueSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Port, externalMySQLSecret, "Port") + assertValueSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Database, externalMySQLSecret, "Database") + assertValueSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Username, externalMySQLSecret, "Username") + assertValueSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.Password, externalMySQLSecret, "Password") + assertEmptyValue(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.SslCa, "mysql sslCa") if cr.Spec.Redis[v2.DefaultInstanceName].ManagedRedis == nil || cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis != nil { t.Fatalf("redis should remain managed") @@ -116,10 +116,10 @@ func TestBuildCRExternalRedisOnly(t *testing.T) { if cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis == nil { t.Fatalf("external redis should be configured") } - assertSelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.Host, externalRedisSecret, "Host") - assertSelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.Port, externalRedisSecret, "Port") - assertEmptySelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.Password, "redis password") - assertEmptySelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.SslCa, "redis sslCa") + assertValueSelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.Host, externalRedisSecret, "Host") + assertValueSelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.Port, externalRedisSecret, "Port") + assertEmptyValue(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.Password, "redis password") + assertEmptyValue(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.SslCa, "redis sslCa") if cr.Spec.MySQL[v2.DefaultInstanceName].ManagedMysql == nil || cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql != nil { t.Fatalf("mysql should remain managed") @@ -141,13 +141,13 @@ func TestBuildCRExternalObjectStoreOnly(t *testing.T) { if cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore == nil { t.Fatalf("external object store should be configured") } - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Provider, externalObjectStoreSecret, "Provider") - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Endpoint, externalObjectStoreSecret, "Host") - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Port, externalObjectStoreSecret, "Port") - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Bucket, externalObjectStoreSecret, "Bucket") - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Region, externalObjectStoreSecret, "Region") - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.AccessKey, externalObjectStoreSecret, "AccessKey") - assertSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.SecretKey, externalObjectStoreSecret, "SecretKey") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Provider, externalObjectStoreSecret, "Provider") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Endpoint, externalObjectStoreSecret, "Host") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Port, externalObjectStoreSecret, "Port") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Bucket, externalObjectStoreSecret, "Bucket") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.Region, externalObjectStoreSecret, "Region") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.AccessKey, externalObjectStoreSecret, "AccessKey") + assertValueSelector(t, cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore.SecretKey, externalObjectStoreSecret, "SecretKey") if cr.Spec.MySQL[v2.DefaultInstanceName].ManagedMysql == nil || cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql != nil { t.Fatalf("mysql should remain managed") @@ -227,8 +227,8 @@ func TestBuildArtifactsExternalInfraWithCustomCA(t *testing.T) { if configMap == nil { t.Fatalf("custom CA ConfigMap should be generated") } - assertSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.SslCa, externalMySQLTLSSecret, "ca.crt") - assertSelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.SslCa, externalRedisTLSSecret, "ca.crt") + assertValueSelector(t, cr.Spec.MySQL[v2.DefaultInstanceName].ExternalMysql.SslCa, externalMySQLTLSSecret, "ca.crt") + assertValueSelector(t, cr.Spec.Redis[v2.DefaultInstanceName].ExternalRedis.SslCa, externalRedisTLSSecret, "ca.crt") if cr.Spec.ObjectStore[v2.DefaultInstanceName].ExternalObjectStore == nil { t.Fatalf("external object store should be configured") } @@ -491,9 +491,18 @@ func assertSelector(t *testing.T, selector corev1.SecretKeySelector, name, key s } } -func assertEmptySelector(t *testing.T, selector corev1.SecretKeySelector, field string) { +func assertValueSelector(t *testing.T, v v2.ValueOrSecret, name, key string) { t.Helper() - if selector.Name != "" || selector.Key != "" { - t.Fatalf("%s selector should be empty, got %s/%s", field, selector.Name, selector.Key) + ref := v.SecretKeyRef() + if ref == nil { + t.Fatalf("selector = , want %s/%s", name, key) + } + assertSelector(t, *ref, name, key) +} + +func assertEmptyValue(t *testing.T, v v2.ValueOrSecret, field string) { + t.Helper() + if ref := v.SecretKeyRef(); ref != nil { + t.Fatalf("%s selector should be empty, got %s/%s", field, ref.Name, ref.Key) } } diff --git a/internal/controller/infra/external/clickhouse/clickhouse.go b/internal/controller/infra/external/clickhouse/clickhouse.go index 79e5b2bd..a29162d8 100644 --- a/internal/controller/infra/external/clickhouse/clickhouse.go +++ b/internal/controller/infra/external/clickhouse/clickhouse.go @@ -6,10 +6,8 @@ import ( apiv2 "github.com/wandb/operator/api/v2" "github.com/wandb/operator/internal/controller/infra/external" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -32,7 +30,7 @@ func WriteState( ) []metav1.Condition { logger := ctrl.LoggerFrom(ctx) - fields := map[string]corev1.SecretKeySelector{ + fields := map[string]apiv2.ValueOrSecret{ "url": spec.URL, "Host": spec.Host, "HTTPPort": spec.HTTPPort, @@ -42,7 +40,7 @@ func WriteState( "Database": spec.Database, } - data, err := external.ResolveFields(ctx, c, wandb.Namespace, fields) + data, err := external.ResolveValueFields(ctx, c, wandb.Namespace, fields) if err != nil { logger.Error(err, "failed to resolve external clickhouse fields") return []metav1.Condition{{ @@ -69,15 +67,14 @@ func ReadState( return conditions, nil } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return conditions, &apiv2.ClickHouseConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "url", Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - HTTPPort: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "HTTPPort", Optional: ptr.To(false)}, - TCPPort: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "TCPPort", Optional: ptr.To(false)}, - Username: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "User", Optional: ptr.To(false)}, - Password: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Password", Optional: ptr.To(false)}, - Database: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Database", Optional: ptr.To(false)}, + URL: apiv2.ValueFromSecret(nsName.Name, "url", false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + HTTPPort: apiv2.ValueFromSecret(nsName.Name, "HTTPPort", false), + TCPPort: apiv2.ValueFromSecret(nsName.Name, "TCPPort", false), + Username: apiv2.ValueFromSecret(nsName.Name, "User", false), + Password: apiv2.ValueFromSecret(nsName.Name, "Password", false), + Database: apiv2.ValueFromSecret(nsName.Name, "Database", false), } } diff --git a/internal/controller/infra/external/common.go b/internal/controller/infra/external/common.go index 1fc7cc21..7322d727 100644 --- a/internal/controller/infra/external/common.go +++ b/internal/controller/infra/external/common.go @@ -130,6 +130,40 @@ func ResolveFields( return data, nil } +// ResolveValue returns a ValueOrSecret's literal value, or reads the referenced +// Secret key. An unset value yields "". +func ResolveValue(ctx context.Context, c client.Client, namespace string, v apiv2.ValueOrSecret) (string, error) { + if v.Value != "" { + return v.Value, nil + } + if ref := v.SecretKeyRef(); ref != nil { + return ResolveSecretKey(ctx, c, namespace, *ref) + } + return "", nil +} + +// ResolveValueFields resolves a map of ValueOrSecret into a flat string map, +// dropping fields that resolve to "". It is the ValueOrSecret counterpart of +// ResolveFields. +func ResolveValueFields( + ctx context.Context, + c client.Client, + namespace string, + fields map[string]apiv2.ValueOrSecret, +) (map[string]string, error) { + data := map[string]string{} + for key, v := range fields { + val, err := ResolveValue(ctx, c, namespace, v) + if err != nil { + return nil, fmt.Errorf("field %q: %w", key, err) + } + if val != "" { + data[key] = val + } + } + return data, nil +} + func InferExternalStatus( oldConditions, newConditions []metav1.Condition, generation int64, diff --git a/internal/controller/infra/external/mysql/mysql.go b/internal/controller/infra/external/mysql/mysql.go index 8ed76000..232c0ef3 100644 --- a/internal/controller/infra/external/mysql/mysql.go +++ b/internal/controller/infra/external/mysql/mysql.go @@ -7,10 +7,8 @@ import ( apiv2 "github.com/wandb/operator/api/v2" "github.com/wandb/operator/internal/controller/infra/external" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -39,7 +37,7 @@ func WriteState( ) []metav1.Condition { logger := ctrl.LoggerFrom(ctx) - fields := map[string]corev1.SecretKeySelector{ + fields := map[string]apiv2.ValueOrSecret{ "Host": spec.Host, "Port": spec.Port, "Database": spec.Database, @@ -51,7 +49,7 @@ func WriteState( "SslKey": spec.SslKey, } - data, err := external.ResolveFields(ctx, c, wandb.Namespace, fields) + data, err := external.ResolveValueFields(ctx, c, wandb.Namespace, fields) if err != nil { logger.Error(err, "failed to resolve external mysql fields") return []metav1.Condition{{ @@ -104,18 +102,17 @@ func ReadState( return conditions, nil } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return conditions, &apiv2.MysqlConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "url", Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - Port: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Port", Optional: ptr.To(false)}, - Database: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Database", Optional: ptr.To(false)}, - Username: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Username", Optional: ptr.To(false)}, - Password: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Password", Optional: ptr.To(false)}, - Tls: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Tls", Optional: ptr.To(true)}, - SslCa: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "SslCa", Optional: ptr.To(true)}, - SslCert: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "SslCert", Optional: ptr.To(true)}, - SslKey: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "SslKey", Optional: ptr.To(true)}, + URL: apiv2.ValueFromSecret(nsName.Name, "url", false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + Port: apiv2.ValueFromSecret(nsName.Name, "Port", false), + Database: apiv2.ValueFromSecret(nsName.Name, "Database", false), + Username: apiv2.ValueFromSecret(nsName.Name, "Username", false), + Password: apiv2.ValueFromSecret(nsName.Name, "Password", false), + Tls: apiv2.ValueFromSecret(nsName.Name, "Tls", true), + SslCa: apiv2.ValueFromSecret(nsName.Name, "SslCa", true), + SslCert: apiv2.ValueFromSecret(nsName.Name, "SslCert", true), + SslKey: apiv2.ValueFromSecret(nsName.Name, "SslKey", true), } } diff --git a/internal/controller/infra/external/mysql/mysql_test.go b/internal/controller/infra/external/mysql/mysql_test.go index 77a82965..b15a04d4 100644 --- a/internal/controller/infra/external/mysql/mysql_test.go +++ b/internal/controller/infra/external/mysql/mysql_test.go @@ -16,11 +16,8 @@ import ( const mysqlSourceSecretName = "external-mysql" -func mysqlSel(key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: mysqlSourceSecretName}, - Key: key, - } +func mysqlSel(key string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(mysqlSourceSecretName, key, false) } func TestWriteStateAddsCustomTLSParamsWhenCACertPresent(t *testing.T) { diff --git a/internal/controller/infra/external/objectstore/objectstore.go b/internal/controller/infra/external/objectstore/objectstore.go index 60928521..ad390b9a 100644 --- a/internal/controller/infra/external/objectstore/objectstore.go +++ b/internal/controller/infra/external/objectstore/objectstore.go @@ -10,7 +10,6 @@ import ( apiv2 "github.com/wandb/operator/api/v2" "github.com/wandb/operator/internal/controller/infra/external" osconn "github.com/wandb/operator/internal/controller/infra/objectstore" - 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" @@ -40,7 +39,7 @@ func WriteState( ) ([]metav1.Condition, *apiv2.ObjectStoreConnection) { logger := ctrl.LoggerFrom(ctx) - fields := map[string]corev1.SecretKeySelector{ + fields := map[string]apiv2.ValueOrSecret{ "Host": spec.Endpoint, "Port": spec.Port, "AccessKey": spec.AccessKey, @@ -53,7 +52,7 @@ func WriteState( "ForcePathStyle": spec.ForcePathStyle, } - data, err := external.ResolveFields(ctx, c, wandb.Namespace, fields) + data, err := external.ResolveValueFields(ctx, c, wandb.Namespace, fields) if err != nil { logger.Error(err, "failed to resolve external object store fields") return []metav1.Condition{{ diff --git a/internal/controller/infra/external/objectstore/objectstore_test.go b/internal/controller/infra/external/objectstore/objectstore_test.go index 84630d75..777c30c5 100644 --- a/internal/controller/infra/external/objectstore/objectstore_test.go +++ b/internal/controller/infra/external/objectstore/objectstore_test.go @@ -16,11 +16,8 @@ import ( const sourceSecretName = "ext-objectstore" -func sel(key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: sourceSecretName}, - Key: key, - } +func sel(key string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(sourceSecretName, key, false) } // writeStateFixture builds a fake client seeded with a source Secret holding @@ -183,14 +180,56 @@ func TestWriteState_FullConfig(t *testing.T) { require.Equal(t, "us-west-2", data["Region"]) // Optional flags: only url and Bucket are required. - require.NotNil(t, conn.URL.Optional) - require.False(t, *conn.URL.Optional) - require.NotNil(t, conn.Bucket.Optional) - require.False(t, *conn.Bucket.Optional) - for _, s := range []corev1.SecretKeySelector{conn.Endpoint, conn.Port, conn.AccessKey, conn.SecretKey, conn.Region} { - require.NotNil(t, s.Optional) - require.True(t, *s.Optional) + require.NotNil(t, conn.URL.SecretKeyRef()) + require.False(t, *conn.URL.SecretKeyRef().Optional) + require.NotNil(t, conn.Bucket.SecretKeyRef()) + require.False(t, *conn.Bucket.SecretKeyRef().Optional) + for _, s := range []apiv2.ValueOrSecret{conn.Endpoint, conn.Port, conn.AccessKey, conn.SecretKey, conn.Region} { + ref := s.SecretKeyRef() + require.NotNil(t, ref) + require.NotNil(t, ref.Optional) + require.True(t, *ref.Optional) + } +} + +// TestWriteState_LiteralValues proves the value-or-secret union: non-secret +// fields are supplied as plain literals while credentials stay in a Secret, and +// both resolve into the operator connection secret. +func TestWriteState_LiteralValues(t *testing.T) { + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, apiv2.AddToScheme(scheme)) + + source := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: sourceSecretName, Namespace: "default"}, + Data: map[string][]byte{"AccessKey": []byte("access"), "SecretKey": []byte("secret")}, + } + ext := &apiv2.ObjectStoreConnection{ + Endpoint: apiv2.LiteralValue("s3.us-west-2.amazonaws.com"), + Port: apiv2.LiteralValue("443"), + Bucket: apiv2.LiteralValue("my-wandb-bucket"), + Region: apiv2.LiteralValue("us-west-2"), + AccessKey: apiv2.ValueFromSecret(sourceSecretName, "AccessKey", false), + SecretKey: apiv2.ValueFromSecret(sourceSecretName, "SecretKey", false), + } + wandb := &apiv2.WeightsAndBiases{ + ObjectMeta: metav1.ObjectMeta{Name: "wandb", Namespace: "default"}, + Spec: apiv2.WeightsAndBiasesSpec{ObjectStore: map[string]apiv2.ObjectStoreSpec{ + apiv2.DefaultInstanceName: {ExternalObjectStore: ext}, + }}, } + + c := fake.NewClientBuilder().WithScheme(scheme).WithObjects(wandb, source).Build() + conditions, conn := WriteState(context.Background(), c, wandb, apiv2.DefaultInstanceName, ext) + require.Nil(t, conditions) + require.NotNil(t, conn) + + written := &corev1.Secret{} + require.NoError(t, c.Get(context.Background(), types.NamespacedName{Name: ConnectionSecretName, Namespace: "default"}, written)) + data := connectionData(written) + require.Equal(t, "us-west-2", data["Region"]) + require.Equal(t, "my-wandb-bucket", data["Bucket"]) + require.Equal(t, "s3://access:secret@s3.us-west-2.amazonaws.com:443/my-wandb-bucket", data["url"]) } func TestWriteState_PathPrefix(t *testing.T) { @@ -210,9 +249,9 @@ func TestWriteState_PathPrefix(t *testing.T) { data := connectionData(written) require.Equal(t, "s3://minio:minio123@minio.local:9000/my-bucket/team/prefix", data["url"]) require.Equal(t, "team/prefix", data["Path"]) - require.Equal(t, "Path", conn.Path.Key) - require.NotNil(t, conn.Path.Optional) - require.True(t, *conn.Path.Optional) + require.Equal(t, "Path", conn.Path.SecretKeyRef().Key) + require.NotNil(t, conn.Path.SecretKeyRef().Optional) + require.True(t, *conn.Path.SecretKeyRef().Optional) // Native AWS with a prefix; slashes are normalized. _, written, conditions, _ = writeStateFixture(t, @@ -284,8 +323,8 @@ func TestWriteState_GCSWorkloadIdentity(t *testing.T) { ) require.Nil(t, conditions) require.NotNil(t, conn) - require.Equal(t, ConnectionSecretName, conn.Provider.Name) - require.Equal(t, "Provider", conn.Provider.Key) + require.Equal(t, ConnectionSecretName, conn.Provider.SecretKeyRef().Name) + require.Equal(t, "Provider", conn.Provider.SecretKeyRef().Key) data := connectionData(written) require.Equal(t, "gs://my-gcs-bucket", data["url"], "workload identity carries no credentials") @@ -319,8 +358,8 @@ func TestWriteState_AzureWithKey(t *testing.T) { ) require.Nil(t, conditions) require.NotNil(t, conn) - require.Equal(t, ConnectionSecretName, conn.Provider.Name) - require.Equal(t, "Provider", conn.Provider.Key) + require.Equal(t, ConnectionSecretName, conn.Provider.SecretKeyRef().Name) + require.Equal(t, "Provider", conn.Provider.SecretKeyRef().Key) data := connectionData(written) require.Equal(t, "az://:accountkey==@mystorageaccount/mycontainer", data["url"]) diff --git a/internal/controller/infra/external/redis/redis.go b/internal/controller/infra/external/redis/redis.go index be9193e1..93c4593e 100644 --- a/internal/controller/infra/external/redis/redis.go +++ b/internal/controller/infra/external/redis/redis.go @@ -10,10 +10,8 @@ import ( apiv2 "github.com/wandb/operator/api/v2" "github.com/wandb/operator/internal/controller/common" "github.com/wandb/operator/internal/controller/infra/external" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -37,7 +35,7 @@ func WriteState( ) []metav1.Condition { logger := ctrl.LoggerFrom(ctx) - fields := map[string]corev1.SecretKeySelector{ + fields := map[string]apiv2.ValueOrSecret{ "Host": spec.Host, "Port": spec.Port, "Password": spec.Password, @@ -45,7 +43,7 @@ func WriteState( "SslCa": spec.SslCa, } - data, err := external.ResolveFields(ctx, c, wandb.Namespace, fields) + data, err := external.ResolveValueFields(ctx, c, wandb.Namespace, fields) if err != nil { logger.Error(err, "failed to resolve external redis fields") return []metav1.Condition{{ @@ -125,14 +123,13 @@ func ReadState( return conditions, nil } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return conditions, &apiv2.RedisConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "url", Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - Port: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Port", Optional: ptr.To(false)}, - Password: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Password", Optional: ptr.To(true)}, - Tls: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Tls", Optional: ptr.To(true)}, - SslCa: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "SslCa", Optional: ptr.To(true)}, + URL: apiv2.ValueFromSecret(nsName.Name, "url", false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + Port: apiv2.ValueFromSecret(nsName.Name, "Port", false), + Password: apiv2.ValueFromSecret(nsName.Name, "Password", true), + Tls: apiv2.ValueFromSecret(nsName.Name, "Tls", true), + SslCa: apiv2.ValueFromSecret(nsName.Name, "SslCa", true), } } diff --git a/internal/controller/infra/external/redis/redis_test.go b/internal/controller/infra/external/redis/redis_test.go index 8a694bc8..0e448fd2 100644 --- a/internal/controller/infra/external/redis/redis_test.go +++ b/internal/controller/infra/external/redis/redis_test.go @@ -19,11 +19,8 @@ import ( const redisSourceSecretName = "external-redis" -func redisSel(key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: redisSourceSecretName}, - Key: key, - } +func redisSel(key string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(redisSourceSecretName, key, false) } func redisWriteStateFixture(t *testing.T, sourceData map[string][]byte) (ctrlclient.Client, *apiv2.WeightsAndBiases) { diff --git a/internal/controller/infra/managed/clickhouse/altinity/conn.go b/internal/controller/infra/managed/clickhouse/altinity/conn.go index 1a64641f..70a0cb25 100644 --- a/internal/controller/infra/managed/clickhouse/altinity/conn.go +++ b/internal/controller/infra/managed/clickhouse/altinity/conn.go @@ -104,14 +104,13 @@ func writeClickHouseConnInfo( return nil, err } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return &apiv2.ClickHouseConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: urlKey, Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - HTTPPort: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "HTTPPort", Optional: ptr.To(false)}, - TCPPort: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "TCPPort", Optional: ptr.To(false)}, - Username: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "User", Optional: ptr.To(false)}, - Password: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Password", Optional: ptr.To(false)}, - Database: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Database", Optional: ptr.To(false)}, + URL: apiv2.ValueFromSecret(nsName.Name, urlKey, false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + HTTPPort: apiv2.ValueFromSecret(nsName.Name, "HTTPPort", false), + TCPPort: apiv2.ValueFromSecret(nsName.Name, "TCPPort", false), + Username: apiv2.ValueFromSecret(nsName.Name, "User", false), + Password: apiv2.ValueFromSecret(nsName.Name, "Password", false), + Database: apiv2.ValueFromSecret(nsName.Name, "Database", false), }, nil } diff --git a/internal/controller/infra/managed/kafka/bufstream/conn.go b/internal/controller/infra/managed/kafka/bufstream/conn.go index e0239dac..39d8a9ed 100644 --- a/internal/controller/infra/managed/kafka/bufstream/conn.go +++ b/internal/controller/infra/managed/kafka/bufstream/conn.go @@ -89,11 +89,10 @@ func writeKafkaConnInfo( return nil, err } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return &apiv2.KafkaConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: urlKey, Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - Port: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Port", Optional: ptr.To(false)}, - BrokerEndpoint: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, + URL: apiv2.ValueFromSecret(nsName.Name, urlKey, false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + Port: apiv2.ValueFromSecret(nsName.Name, "Port", false), + BrokerEndpoint: apiv2.ValueFromSecret(nsName.Name, "Host", false), }, nil } diff --git a/internal/controller/infra/managed/kafka/bufstream/write_test.go b/internal/controller/infra/managed/kafka/bufstream/write_test.go index c1b78b03..54d4e588 100644 --- a/internal/controller/infra/managed/kafka/bufstream/write_test.go +++ b/internal/controller/infra/managed/kafka/bufstream/write_test.go @@ -31,11 +31,8 @@ func resolveStorageFixture(t *testing.T, data map[string]string) (ctrlclient.Cli Data: secretData, } - sel := func(key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: secret.Name}, - Key: key, - } + sel := func(key string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(secret.Name, key, false) } wandb := &apiv2.WeightsAndBiases{ ObjectMeta: metav1.ObjectMeta{Name: "wandb", Namespace: "wandb"}, diff --git a/internal/controller/infra/managed/mysql/moco/conn.go b/internal/controller/infra/managed/mysql/moco/conn.go index 918a3b1a..abdade34 100644 --- a/internal/controller/infra/managed/mysql/moco/conn.go +++ b/internal/controller/infra/managed/mysql/moco/conn.go @@ -91,13 +91,12 @@ func writeMySQLConnInfo( return nil, err } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return &apiv2.MysqlConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: urlKey, Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - Port: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Port", Optional: ptr.To(false)}, - Database: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Database", Optional: ptr.To(false)}, - Username: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Username", Optional: ptr.To(false)}, - Password: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Password", Optional: ptr.To(false)}, + URL: apiv2.ValueFromSecret(nsName.Name, urlKey, false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + Port: apiv2.ValueFromSecret(nsName.Name, "Port", false), + Database: apiv2.ValueFromSecret(nsName.Name, "Database", false), + Username: apiv2.ValueFromSecret(nsName.Name, "Username", false), + Password: apiv2.ValueFromSecret(nsName.Name, "Password", false), }, nil } diff --git a/internal/controller/infra/managed/redis/opstree/conn.go b/internal/controller/infra/managed/redis/opstree/conn.go index d527ef14..7d03837b 100644 --- a/internal/controller/infra/managed/redis/opstree/conn.go +++ b/internal/controller/infra/managed/redis/opstree/conn.go @@ -93,10 +93,9 @@ func writeRedisConnInfo( return nil, err } - localRef := corev1.LocalObjectReference{Name: nsName.Name} return &apiv2.RedisConnection{ - URL: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: urlKey, Optional: ptr.To(false)}, - Host: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Host", Optional: ptr.To(false)}, - Port: corev1.SecretKeySelector{LocalObjectReference: localRef, Key: "Port", Optional: ptr.To(false)}, + URL: apiv2.ValueFromSecret(nsName.Name, urlKey, false), + Host: apiv2.ValueFromSecret(nsName.Name, "Host", false), + Port: apiv2.ValueFromSecret(nsName.Name, "Port", false), }, nil } diff --git a/internal/controller/infra/objectstore/resolve_test.go b/internal/controller/infra/objectstore/resolve_test.go index b8103f74..041fde52 100644 --- a/internal/controller/infra/objectstore/resolve_test.go +++ b/internal/controller/infra/objectstore/resolve_test.go @@ -31,11 +31,8 @@ func resolveFixture(t *testing.T, data map[string]string) (*apiv2.ObjectStoreCon Data: raw, } - connSel := func(key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: connSecretName}, - Key: key, - } + connSel := func(key string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(connSecretName, key, false) } conn := &apiv2.ObjectStoreConnection{ Provider: connSel("Provider"), @@ -92,8 +89,8 @@ func TestResolve_ExternalS3WithStaticCredentials(t *testing.T) { require.True(t, info.HasStaticCredentials()) // The credential selectors are preserved for consumers that inject by ref. - require.Equal(t, conn.AccessKey, info.AccessKeyRef) - require.Equal(t, conn.SecretKey, info.SecretKeyRef) + require.Equal(t, *conn.AccessKey.SecretKeyRef(), info.AccessKeyRef) + require.Equal(t, *conn.SecretKey.SecretKeyRef(), info.SecretKeyRef) } func TestResolve_AmbientCredentials(t *testing.T) { diff --git a/internal/controller/infra/objectstore/secret.go b/internal/controller/infra/objectstore/secret.go index 2186439b..098c41a8 100644 --- a/internal/controller/infra/objectstore/secret.go +++ b/internal/controller/infra/objectstore/secret.go @@ -9,7 +9,6 @@ import ( apiv2 "github.com/wandb/operator/api/v2" "github.com/wandb/operator/pkg/utils" corev1 "k8s.io/api/core/v1" - "k8s.io/utils/ptr" "sigs.k8s.io/controller-runtime/pkg/client" ) @@ -53,11 +52,10 @@ func (c ConnInfo) ToSecretData() map[string]string { // the rest are optional (external configs omit provider-dependent keys). func (c ConnInfo) ToObjectStoreConnection(secretName string, requireAll bool) *apiv2.ObjectStoreConnection { data := c.ToSecretData() - localRef := corev1.LocalObjectReference{Name: secretName} - sel := func(key string) corev1.SecretKeySelector { + sel := func(key string) apiv2.ValueOrSecret { optional := !requireAll && !connectionRequiredKeys[key] - return corev1.SecretKeySelector{LocalObjectReference: localRef, Key: key, Optional: ptr.To(optional)} + return apiv2.ValueFromSecret(secretName, key, optional) } has := func(key string) bool { _, ok := data[key]; return ok } @@ -111,12 +109,17 @@ func Resolve( resolver := &utils.ConnSecretResolver{Client: cl, Namespace: namespace, Cache: map[string]*corev1.Secret{}} - info := ConnInfo{ - AccessKeyRef: conn.AccessKey, - SecretKeyRef: conn.SecretKey, + info := ConnInfo{} + // Status connections always carry the secret arm; capture the selectors for + // consumers that inject creds by reference (ClickHouse disk, Bufstream). + if ref := conn.AccessKey.SecretKeyRef(); ref != nil { + info.AccessKeyRef = *ref + } + if ref := conn.SecretKey.SecretKeyRef(); ref != nil { + info.SecretKeyRef = *ref } - provider, err := resolver.Value(ctx, conn.Provider) + provider, err := resolver.ValueOrSecret(ctx, conn.Provider) if err != nil { return ConnInfo{}, err } @@ -127,33 +130,33 @@ func Resolve( } info.Provider = apiv2.ObjectStoreProvider(provider) - if info.Bucket, err = resolver.Value(ctx, conn.Bucket); err != nil { + if info.Bucket, err = resolver.ValueOrSecret(ctx, conn.Bucket); err != nil { return ConnInfo{}, err } - if info.Endpoint, err = resolver.Value(ctx, conn.Endpoint); err != nil { + if info.Endpoint, err = resolver.ValueOrSecret(ctx, conn.Endpoint); err != nil { return ConnInfo{}, err } - if info.Port, err = resolver.Value(ctx, conn.Port); err != nil { + if info.Port, err = resolver.ValueOrSecret(ctx, conn.Port); err != nil { return ConnInfo{}, err } - if info.Region, err = resolver.Value(ctx, conn.Region); err != nil { + if info.Region, err = resolver.ValueOrSecret(ctx, conn.Region); err != nil { return ConnInfo{}, err } - if info.AccessKey, err = resolver.Value(ctx, conn.AccessKey); err != nil { + if info.AccessKey, err = resolver.ValueOrSecret(ctx, conn.AccessKey); err != nil { return ConnInfo{}, err } - if info.SecretKey, err = resolver.Value(ctx, conn.SecretKey); err != nil { + if info.SecretKey, err = resolver.ValueOrSecret(ctx, conn.SecretKey); err != nil { return ConnInfo{}, err } // A half-configured pair silently picks the wrong credential mode downstream. if (info.AccessKey == "") != (info.SecretKey == "") { return ConnInfo{}, fmt.Errorf("object store access key and secret key must be configured together") } - if info.Path, err = resolver.Value(ctx, conn.Path); err != nil { + if info.Path, err = resolver.ValueOrSecret(ctx, conn.Path); err != nil { return ConnInfo{}, err } - forcePathStyleString, err := resolver.Value(ctx, conn.ForcePathStyle) + forcePathStyleString, err := resolver.ValueOrSecret(ctx, conn.ForcePathStyle) if err != nil { return ConnInfo{}, err } @@ -164,7 +167,7 @@ func Resolve( info.ForcePathStyle = RequiresPathStyle(info.Endpoint) } - tlsEnabledString, err := resolver.Value(ctx, conn.TlsEnabled) + tlsEnabledString, err := resolver.ValueOrSecret(ctx, conn.TlsEnabled) if err != nil { return ConnInfo{}, err } diff --git a/internal/controller/infra/objectstore/secret_test.go b/internal/controller/infra/objectstore/secret_test.go index 9e0883ad..36d24d61 100644 --- a/internal/controller/infra/objectstore/secret_test.go +++ b/internal/controller/infra/objectstore/secret_test.go @@ -4,7 +4,6 @@ import ( "testing" "github.com/stretchr/testify/require" - corev1 "k8s.io/api/core/v1" apiv2 "github.com/wandb/operator/api/v2" ) @@ -83,18 +82,20 @@ func TestToObjectStoreConnection_RequireAll(t *testing.T) { conn := ci.ToObjectStoreConnection("conn-secret", true) // Every emitted selector points at conn-secret and is required. - for _, s := range []corev1.SecretKeySelector{ + for _, s := range []apiv2.ValueOrSecret{ conn.URL, conn.Provider, conn.Endpoint, conn.Port, conn.AccessKey, conn.SecretKey, conn.Region, conn.Bucket, conn.TlsEnabled, conn.ForcePathStyle, } { - require.Equal(t, "conn-secret", s.Name) - require.NotNil(t, s.Optional) - require.False(t, *s.Optional) + ref := s.SecretKeyRef() + require.NotNil(t, ref) + require.Equal(t, "conn-secret", ref.Name) + require.NotNil(t, ref.Optional) + require.False(t, *ref.Optional) } - require.Equal(t, "Host", conn.Endpoint.Key) - require.Equal(t, "url", conn.URL.Key) + require.Equal(t, "Host", conn.Endpoint.SecretKeyRef().Key) + require.Equal(t, "url", conn.URL.SecretKeyRef().Key) // Path is not written for the managed shape, so its selector stays empty. - require.Empty(t, conn.Path.Name) + require.Nil(t, conn.Path.SecretKeyRef()) } func TestToObjectStoreConnection_ExternalOptionality(t *testing.T) { @@ -111,13 +112,17 @@ func TestToObjectStoreConnection_ExternalOptionality(t *testing.T) { conn := ci.ToObjectStoreConnection("conn-secret", false) // url/Provider/Bucket are required... - for _, s := range []corev1.SecretKeySelector{conn.URL, conn.Provider, conn.Bucket} { - require.NotNil(t, s.Optional) - require.False(t, *s.Optional) + for _, s := range []apiv2.ValueOrSecret{conn.URL, conn.Provider, conn.Bucket} { + ref := s.SecretKeyRef() + require.NotNil(t, ref) + require.NotNil(t, ref.Optional) + require.False(t, *ref.Optional) } // ...everything else is optional. - for _, s := range []corev1.SecretKeySelector{conn.Endpoint, conn.Port, conn.AccessKey, conn.SecretKey, conn.Region} { - require.NotNil(t, s.Optional) - require.True(t, *s.Optional) + for _, s := range []apiv2.ValueOrSecret{conn.Endpoint, conn.Port, conn.AccessKey, conn.SecretKey, conn.Region} { + ref := s.SecretKeyRef() + require.NotNil(t, ref) + require.NotNil(t, ref.Optional) + require.True(t, *ref.Optional) } } diff --git a/internal/controller/reconciler/custom_ca.go b/internal/controller/reconciler/custom_ca.go index 8dfe6c31..3102db87 100644 --- a/internal/controller/reconciler/custom_ca.go +++ b/internal/controller/reconciler/custom_ca.go @@ -286,14 +286,18 @@ func setCustomCACertsChecksumAnnotation(podTemplate *corev1.PodTemplateSpec, che podTemplate.SetAnnotations(annotations) } -func secretCACertVolumeSource(sel corev1.SecretKeySelector, fileName string) *corev1.SecretVolumeSource { +func secretCACertVolumeSource(v apiv2.ValueOrSecret, fileName string) *corev1.SecretVolumeSource { + ref := v.SecretKeyRef() + if ref == nil { + return nil + } return &corev1.SecretVolumeSource{ - SecretName: sel.Name, + SecretName: ref.Name, Items: []corev1.KeyToPath{{ - Key: sel.Key, + Key: ref.Key, Path: fileName, }}, - Optional: sel.Optional, + Optional: ref.Optional, } } @@ -317,7 +321,12 @@ func upsertVolumeMount(volumeMounts []corev1.VolumeMount, mount corev1.VolumeMou return append(volumeMounts, mount) } -func secretSelectorHasValue(ctx context.Context, c ctrlClient.Client, namespace string, sel corev1.SecretKeySelector) (bool, error) { +func secretSelectorHasValue(ctx context.Context, c ctrlClient.Client, namespace string, v apiv2.ValueOrSecret) (bool, error) { + ref := v.SecretKeyRef() + if ref == nil { + return false, nil + } + sel := *ref if !secretSelectorConfigured(sel) { return false, nil } @@ -377,28 +386,28 @@ func customCACertsChecksum(ctx context.Context, c ctrlClient.Client, wandb *apiv } } - if sel := mysqlConn.SslCa; hasMySQLCA { - _, _ = fmt.Fprintf(hash, "mysql:%s/%s\n", sel.Name, sel.Key) - if err := hashSecretKeyData(ctx, c, wandb.Namespace, sel, hashWriteString(hash)); err != nil { + if ref := mysqlConn.SslCa.SecretKeyRef(); hasMySQLCA && ref != nil { + _, _ = fmt.Fprintf(hash, "mysql:%s/%s\n", ref.Name, ref.Key) + if err := hashSecretKeyData(ctx, c, wandb.Namespace, *ref, hashWriteString(hash)); err != nil { return "", err } } - if sel := mysqlConn.SslCert; hasMySQLCert { - _, _ = fmt.Fprintf(hash, "mysql-cert:%s/%s\n", sel.Name, sel.Key) - if err := hashSecretKeyData(ctx, c, wandb.Namespace, sel, hashWriteString(hash)); err != nil { + if ref := mysqlConn.SslCert.SecretKeyRef(); hasMySQLCert && ref != nil { + _, _ = fmt.Fprintf(hash, "mysql-cert:%s/%s\n", ref.Name, ref.Key) + if err := hashSecretKeyData(ctx, c, wandb.Namespace, *ref, hashWriteString(hash)); err != nil { return "", err } } - if sel := mysqlConn.SslKey; hasMySQLKey { - _, _ = fmt.Fprintf(hash, "mysql-key:%s/%s\n", sel.Name, sel.Key) - if err := hashSecretKeyData(ctx, c, wandb.Namespace, sel, hashWriteString(hash)); err != nil { + if ref := mysqlConn.SslKey.SecretKeyRef(); hasMySQLKey && ref != nil { + _, _ = fmt.Fprintf(hash, "mysql-key:%s/%s\n", ref.Name, ref.Key) + if err := hashSecretKeyData(ctx, c, wandb.Namespace, *ref, hashWriteString(hash)); err != nil { return "", err } } - if sel := redisConn.SslCa; hasRedisCA { - _, _ = fmt.Fprintf(hash, "redis:%s/%s\n", sel.Name, sel.Key) - if err := hashSecretKeyData(ctx, c, wandb.Namespace, sel, hashWriteString(hash)); err != nil { + if ref := redisConn.SslCa.SecretKeyRef(); hasRedisCA && ref != nil { + _, _ = fmt.Fprintf(hash, "redis:%s/%s\n", ref.Name, ref.Key) + if err := hashSecretKeyData(ctx, c, wandb.Namespace, *ref, hashWriteString(hash)); err != nil { return "", err } } diff --git a/internal/controller/reconciler/custom_ca_test.go b/internal/controller/reconciler/custom_ca_test.go index ea43e3df..edfc76b1 100644 --- a/internal/controller/reconciler/custom_ca_test.go +++ b/internal/controller/reconciler/custom_ca_test.go @@ -10,7 +10,6 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" - "k8s.io/utils/ptr" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -95,29 +94,16 @@ func TestApplyCustomCACertsToWorkloadAddsGlobalAndInfraMounts(t *testing.T) { MySQLStatus: map[string]apiv2.MysqlInfraStatus{ apiv2.DefaultInstanceName: apiv2.MysqlInfraStatus{ Connection: apiv2.MysqlConnection{ - SslCa: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "wandb-mysql-connection"}, - Key: "SslCa", - }, - SslCert: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "wandb-mysql-connection"}, - Key: "SslCert", - }, - SslKey: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "wandb-mysql-connection"}, - Key: "SslKey", - }, + SslCa: apiv2.ValueFromSecret("wandb-mysql-connection", "SslCa", false), + SslCert: apiv2.ValueFromSecret("wandb-mysql-connection", "SslCert", false), + SslKey: apiv2.ValueFromSecret("wandb-mysql-connection", "SslKey", false), }, }, }, RedisStatus: map[string]apiv2.RedisInfraStatus{ apiv2.DefaultInstanceName: apiv2.RedisInfraStatus{ Connection: apiv2.RedisConnection{ - SslCa: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "wandb-redis-connection"}, - Key: "SslCa", - Optional: ptr.To(true), - }, + SslCa: apiv2.ValueFromSecret("wandb-redis-connection", "SslCa", true), }, }, }, @@ -182,22 +168,14 @@ func TestApplyCustomCACertsToWorkloadSkipsMissingOptionalInfraKeys(t *testing.T) MySQLStatus: map[string]apiv2.MysqlInfraStatus{ apiv2.DefaultInstanceName: apiv2.MysqlInfraStatus{ Connection: apiv2.MysqlConnection{ - SslCa: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "wandb-mysql-connection"}, - Key: "SslCa", - Optional: ptr.To(true), - }, + SslCa: apiv2.ValueFromSecret("wandb-mysql-connection", "SslCa", true), }, }, }, RedisStatus: map[string]apiv2.RedisInfraStatus{ apiv2.DefaultInstanceName: apiv2.RedisInfraStatus{ Connection: apiv2.RedisConnection{ - SslCa: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "wandb-redis-connection"}, - Key: "SslCa", - Optional: ptr.To(true), - }, + SslCa: apiv2.ValueFromSecret("wandb-redis-connection", "SslCa", true), }, }, }, diff --git a/internal/controller/reconciler/kafka.go b/internal/controller/reconciler/kafka.go index e99b9d6e..8e72e0ef 100644 --- a/internal/controller/reconciler/kafka.go +++ b/internal/controller/reconciler/kafka.go @@ -239,10 +239,11 @@ func createTopicIdempotent(ctx context.Context, admin *kadm.Client, topicName st // in-cluster broker host:port used by the admin client. func resolveKafkaBootstrap(ctx context.Context, cl client.Client, wandb *apiv2.WeightsAndBiases) (string, error) { conn := wandb.Status.KafkaStatus.Connection - secretName := conn.Host.Name - if secretName == "" { + ref := conn.Host.SecretKeyRef() + if ref == nil || ref.Name == "" { return "", fmt.Errorf("kafka connection secret not set in status") } + secretName := ref.Name spec := wandb.Spec.Kafka.ManagedKafka secret := &corev1.Secret{} diff --git a/internal/controller/reconciler/migrate_legacy.go b/internal/controller/reconciler/migrate_legacy.go index c3557bd5..3a0eeee2 100644 --- a/internal/controller/reconciler/migrate_legacy.go +++ b/internal/controller/reconciler/migrate_legacy.go @@ -111,12 +111,12 @@ func migrateLegacyMySQL( } data := map[string][]byte{} - fill := func(target *corev1.SecretKeySelector, dataKey, value string) { - if target.Name != "" || value == "" { + fill := func(target *apiv2.ValueOrSecret, dataKey, value string) { + if !target.IsZero() || value == "" { return } data[dataKey] = []byte(value) - *target = secretSelector(secretName, dataKey) + *target = apiv2.ValueFromSecret(secretName, dataKey, false) } fill(&conn.Host, "host", payload.Host) @@ -171,12 +171,12 @@ func migrateLegacyRedis( } data := map[string][]byte{} - fill := func(target *corev1.SecretKeySelector, dataKey, value string) { - if target.Name != "" || value == "" { + fill := func(target *apiv2.ValueOrSecret, dataKey, value string) { + if !target.IsZero() || value == "" { return } data[dataKey] = []byte(value) - *target = secretSelector(secretName, dataKey) + *target = apiv2.ValueFromSecret(secretName, dataKey, false) } fill(&conn.Host, "host", payload.Host) @@ -230,12 +230,12 @@ func migrateLegacyClickHouse( } data := map[string][]byte{} - fill := func(target *corev1.SecretKeySelector, dataKey, value string) { - if target.Name != "" || value == "" { + fill := func(target *apiv2.ValueOrSecret, dataKey, value string) { + if !target.IsZero() || value == "" { return } data[dataKey] = []byte(value) - *target = secretSelector(secretName, dataKey) + *target = apiv2.ValueFromSecret(secretName, dataKey, false) } fill(&conn.Host, "host", payload.Host) @@ -297,12 +297,12 @@ func migrateLegacyBucket( forcePathStyle, tlsEnabled := deriveBucketAddressing(payload.Provider, endpoint, query) data := map[string][]byte{} - fill := func(target *corev1.SecretKeySelector, dataKey, value string) { - if target.Name != "" || value == "" { + fill := func(target *apiv2.ValueOrSecret, dataKey, value string) { + if !target.IsZero() || value == "" { return } data[dataKey] = []byte(value) - *target = secretSelector(secretName, dataKey) + *target = apiv2.ValueFromSecret(secretName, dataKey, false) } fill(&conn.Endpoint, "endpoint", endpoint) @@ -405,12 +405,12 @@ func migrateLegacyOIDC( oidc := &wandb.Spec.Wandb.OIDC data := map[string][]byte{} - fill := func(target *corev1.SecretKeySelector, dataKey, value string) { - if target.Name != "" || value == "" { + fill := func(target *apiv2.ValueOrSecret, dataKey, value string) { + if !target.IsZero() || value == "" { return } data[dataKey] = []byte(value) - *target = secretSelector(secretName, dataKey) + *target = apiv2.ValueFromSecret(secretName, dataKey, false) } fill(&oidc.ClientId, "clientId", payload.ClientId) @@ -472,10 +472,3 @@ func normalizePort(v any) string { return fmt.Sprintf("%v", p) } } - -func secretSelector(name, key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } -} diff --git a/internal/controller/reconciler/migrate_legacy_test.go b/internal/controller/reconciler/migrate_legacy_test.go index 6a16d569..b8c278d8 100644 --- a/internal/controller/reconciler/migrate_legacy_test.go +++ b/internal/controller/reconciler/migrate_legacy_test.go @@ -131,17 +131,17 @@ func TestMigrateLegacyMySQL_FullLiteralPayload(t *testing.T) { require.NotContains(t, fresh.Annotations, apiv1.MySQLPendingAnnotation) require.NotNil(t, fresh.Spec.MySQL[apiv2.DefaultInstanceName].ExternalMysql) conn := fresh.Spec.MySQL[apiv2.DefaultInstanceName].ExternalMysql - require.Equal(t, "wandb-mysql-converted", conn.Host.Name) - require.Equal(t, "host", conn.Host.Key) - require.Equal(t, "port", conn.Port.Key) - require.Equal(t, "database", conn.Database.Key) - require.Equal(t, "username", conn.Username.Key) - require.Equal(t, "password", conn.Password.Key) - require.Equal(t, "sslCa", conn.SslCa.Key) - require.Empty(t, conn.Tls.Name) - require.Empty(t, conn.SslCert.Name) - require.Empty(t, conn.SslKey.Name) - require.Empty(t, conn.URL.Name) + require.Equal(t, "wandb-mysql-converted", conn.Host.SecretKeyRef().Name) + require.Equal(t, "host", conn.Host.SecretKeyRef().Key) + require.Equal(t, "port", conn.Port.SecretKeyRef().Key) + require.Equal(t, "database", conn.Database.SecretKeyRef().Key) + require.Equal(t, "username", conn.Username.SecretKeyRef().Key) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) + require.Equal(t, "sslCa", conn.SslCa.SecretKeyRef().Key) + require.Nil(t, conn.Tls.SecretKeyRef()) + require.Nil(t, conn.SslCert.SecretKeyRef()) + require.Nil(t, conn.SslKey.SecretKeyRef()) + require.Nil(t, conn.URL.SecretKeyRef()) } func TestMigrateLegacyMySQL_PartialPayload(t *testing.T) { @@ -165,12 +165,12 @@ func TestMigrateLegacyMySQL_PartialPayload(t *testing.T) { conn := wandb.Spec.MySQL[apiv2.DefaultInstanceName].ExternalMysql require.NotNil(t, conn) - require.Equal(t, "host", conn.Host.Key) - require.Equal(t, "password", conn.Password.Key) - require.Empty(t, conn.Port.Name) - require.Empty(t, conn.Database.Name) - require.Empty(t, conn.Username.Name) - require.Empty(t, conn.SslCa.Name) + require.Equal(t, "host", conn.Host.SecretKeyRef().Key) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) + require.Nil(t, conn.Port.SecretKeyRef()) + require.Nil(t, conn.Database.SecretKeyRef()) + require.Nil(t, conn.Username.SecretKeyRef()) + require.Nil(t, conn.SslCa.SecretKeyRef()) } func TestMigrateLegacyMySQL_PreSetFieldsAreRespected(t *testing.T) { @@ -181,10 +181,7 @@ func TestMigrateLegacyMySQL_PreSetFieldsAreRespected(t *testing.T) { w.Spec.MySQL = map[string]apiv2.MySQLSpec{ apiv2.DefaultInstanceName: { ExternalMysql: &apiv2.MysqlConnection{ - Host: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset-secret"}, - Key: "preset-host-key", - }, + Host: apiv2.ValueFromSecret("preset-secret", "preset-host-key", false), }, }, } @@ -201,10 +198,10 @@ func TestMigrateLegacyMySQL_PreSetFieldsAreRespected(t *testing.T) { require.Contains(t, secret.Data, "database") conn := wandb.Spec.MySQL[apiv2.DefaultInstanceName].ExternalMysql - require.Equal(t, "preset-secret", conn.Host.Name) - require.Equal(t, "preset-host-key", conn.Host.Key) - require.Equal(t, "wandb-mysql-converted", conn.Port.Name) - require.Equal(t, "wandb-mysql-converted", conn.Database.Name) + require.Equal(t, "preset-secret", conn.Host.SecretKeyRef().Name) + require.Equal(t, "preset-host-key", conn.Host.SecretKeyRef().Key) + require.Equal(t, "wandb-mysql-converted", conn.Port.SecretKeyRef().Name) + require.Equal(t, "wandb-mysql-converted", conn.Database.SecretKeyRef().Name) } func TestMigrateLegacyMySQL_AllPreSetEmptyAnnotationPayload(t *testing.T) { @@ -215,10 +212,7 @@ func TestMigrateLegacyMySQL_AllPreSetEmptyAnnotationPayload(t *testing.T) { w.Spec.MySQL = map[string]apiv2.MySQLSpec{ apiv2.DefaultInstanceName: { ExternalMysql: &apiv2.MysqlConnection{ - Host: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset"}, - Key: "host", - }, + Host: apiv2.ValueFromSecret("preset", "host", false), }, }, } @@ -234,7 +228,7 @@ func TestMigrateLegacyMySQL_AllPreSetEmptyAnnotationPayload(t *testing.T) { var fresh apiv2.WeightsAndBiases require.NoError(t, client.Get(context.Background(), types.NamespacedName{Name: "wandb", Namespace: "default"}, &fresh)) require.NotContains(t, fresh.Annotations, apiv1.MySQLPendingAnnotation) - require.Equal(t, "preset", fresh.Spec.MySQL[apiv2.DefaultInstanceName].ExternalMysql.Host.Name) + require.Equal(t, "preset", fresh.Spec.MySQL[apiv2.DefaultInstanceName].ExternalMysql.Host.SecretKeyRef().Name) } func TestMigrateLegacyMySQL_PreExistingSecretOverwritten(t *testing.T) { @@ -318,13 +312,13 @@ func TestMigrateLegacyRedis_FullLiteralPayload(t *testing.T) { require.NotContains(t, fresh.Annotations, apiv1.RedisPendingAnnotation) conn := fresh.Spec.Redis[apiv2.DefaultInstanceName].ExternalRedis require.NotNil(t, conn) - require.Equal(t, "wandb-redis-converted", conn.Host.Name) - require.Equal(t, "host", conn.Host.Key) - require.Equal(t, "port", conn.Port.Key) - require.Equal(t, "password", conn.Password.Key) - require.Equal(t, "sslCa", conn.SslCa.Key) - require.Equal(t, "tls", conn.Tls.Key) - require.Empty(t, conn.URL.Name) + require.Equal(t, "wandb-redis-converted", conn.Host.SecretKeyRef().Name) + require.Equal(t, "host", conn.Host.SecretKeyRef().Key) + require.Equal(t, "port", conn.Port.SecretKeyRef().Key) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) + require.Equal(t, "sslCa", conn.SslCa.SecretKeyRef().Key) + require.Equal(t, "tls", conn.Tls.SecretKeyRef().Key) + require.Nil(t, conn.URL.SecretKeyRef()) } func TestMigrateLegacyRedis_PreSetFieldsAreRespected(t *testing.T) { @@ -335,10 +329,7 @@ func TestMigrateLegacyRedis_PreSetFieldsAreRespected(t *testing.T) { w.Spec.Redis = map[string]apiv2.RedisSpec{ apiv2.DefaultInstanceName: { ExternalRedis: &apiv2.RedisConnection{ - Host: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset-secret"}, - Key: "preset-host-key", - }, + Host: apiv2.ValueFromSecret("preset-secret", "preset-host-key", false), }, }, } @@ -353,9 +344,9 @@ func TestMigrateLegacyRedis_PreSetFieldsAreRespected(t *testing.T) { require.Contains(t, secret.Data, "port") conn := wandb.Spec.Redis[apiv2.DefaultInstanceName].ExternalRedis - require.Equal(t, "preset-secret", conn.Host.Name) - require.Equal(t, "preset-host-key", conn.Host.Key) - require.Equal(t, "wandb-redis-converted", conn.Port.Name) + require.Equal(t, "preset-secret", conn.Host.SecretKeyRef().Name) + require.Equal(t, "preset-host-key", conn.Host.SecretKeyRef().Key) + require.Equal(t, "wandb-redis-converted", conn.Port.SecretKeyRef().Name) } func TestMigrateLegacyRedis_MalformedJSON(t *testing.T) { @@ -432,13 +423,13 @@ func TestMigrateLegacyBucket_BareBucketName(t *testing.T) { conn := wandb.Spec.ObjectStore[apiv2.DefaultInstanceName].ExternalObjectStore require.NotNil(t, conn) - require.Equal(t, "bucket", conn.Bucket.Key) - require.Equal(t, "region", conn.Region.Key) - require.Equal(t, "accessKey", conn.AccessKey.Key) - require.Equal(t, "secretKey", conn.SecretKey.Key) - require.Empty(t, conn.Endpoint.Name) - require.Empty(t, conn.Port.Name) - require.Equal(t, "forcePathStyle", conn.ForcePathStyle.Key) + require.Equal(t, "bucket", conn.Bucket.SecretKeyRef().Key) + require.Equal(t, "region", conn.Region.SecretKeyRef().Key) + require.Equal(t, "accessKey", conn.AccessKey.SecretKeyRef().Key) + require.Equal(t, "secretKey", conn.SecretKey.SecretKeyRef().Key) + require.Nil(t, conn.Endpoint.SecretKeyRef()) + require.Nil(t, conn.Port.SecretKeyRef()) + require.Equal(t, "forcePathStyle", conn.ForcePathStyle.SecretKeyRef().Key) } func TestMigrateLegacyBucket_EmbeddedEndpoint(t *testing.T) { @@ -459,11 +450,11 @@ func TestMigrateLegacyBucket_EmbeddedEndpoint(t *testing.T) { require.Equal(t, []byte("false"), secret.Data["tlsEnabled"], "gorilla defaulted custom endpoints to http") conn := wandb.Spec.ObjectStore[apiv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, "endpoint", conn.Endpoint.Key) - require.Equal(t, "port", conn.Port.Key) - require.Equal(t, "bucket", conn.Bucket.Key) - require.Equal(t, "forcePathStyle", conn.ForcePathStyle.Key) - require.Equal(t, "tlsEnabled", conn.TlsEnabled.Key) + require.Equal(t, "endpoint", conn.Endpoint.SecretKeyRef().Key) + require.Equal(t, "port", conn.Port.SecretKeyRef().Key) + require.Equal(t, "bucket", conn.Bucket.SecretKeyRef().Key) + require.Equal(t, "forcePathStyle", conn.ForcePathStyle.SecretKeyRef().Key) + require.Equal(t, "tlsEnabled", conn.TlsEnabled.SecretKeyRef().Key) } func TestMigrateLegacyBucket_HostPortEndpointWithBucketInPath(t *testing.T) { @@ -479,8 +470,8 @@ func TestMigrateLegacyBucket_HostPortEndpointWithBucketInPath(t *testing.T) { w.Spec.ObjectStore = map[string]apiv2.ObjectStoreSpec{ apiv2.DefaultInstanceName: { ExternalObjectStore: &apiv2.ObjectStoreConnection{ - AccessKey: secretSelector("wandb-minio", "ACCESS_KEY"), - SecretKey: secretSelector("wandb-minio", "SECRET_KEY"), + AccessKey: apiv2.ValueFromSecret("wandb-minio", "ACCESS_KEY", false), + SecretKey: apiv2.ValueFromSecret("wandb-minio", "SECRET_KEY", false), }, }, } @@ -507,15 +498,15 @@ func TestMigrateLegacyBucket_HostPortEndpointWithBucketInPath(t *testing.T) { require.NotContains(t, fresh.Annotations, apiv1.BucketPendingAnnotation) conn := fresh.Spec.ObjectStore[apiv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, secretSelector("wandb-bucket-converted", "endpoint"), conn.Endpoint) - require.Equal(t, secretSelector("wandb-bucket-converted", "port"), conn.Port) - require.Equal(t, secretSelector("wandb-bucket-converted", "bucket"), conn.Bucket) - require.Equal(t, secretSelector("wandb-bucket-converted", "region"), conn.Region) - require.Equal(t, secretSelector("wandb-bucket-converted", "forcePathStyle"), conn.ForcePathStyle) - require.Equal(t, secretSelector("wandb-bucket-converted", "tlsEnabled"), conn.TlsEnabled) - require.Empty(t, conn.Path.Name) - require.Equal(t, secretSelector("wandb-minio", "ACCESS_KEY"), conn.AccessKey) - require.Equal(t, secretSelector("wandb-minio", "SECRET_KEY"), conn.SecretKey) + require.Equal(t, apiv2.ValueFromSecret("wandb-bucket-converted", "endpoint", false), conn.Endpoint) + require.Equal(t, apiv2.ValueFromSecret("wandb-bucket-converted", "port", false), conn.Port) + require.Equal(t, apiv2.ValueFromSecret("wandb-bucket-converted", "bucket", false), conn.Bucket) + require.Equal(t, apiv2.ValueFromSecret("wandb-bucket-converted", "region", false), conn.Region) + require.Equal(t, apiv2.ValueFromSecret("wandb-bucket-converted", "forcePathStyle", false), conn.ForcePathStyle) + require.Equal(t, apiv2.ValueFromSecret("wandb-bucket-converted", "tlsEnabled", false), conn.TlsEnabled) + require.Nil(t, conn.Path.SecretKeyRef()) + require.Equal(t, apiv2.ValueFromSecret("wandb-minio", "ACCESS_KEY", false), conn.AccessKey) + require.Equal(t, apiv2.ValueFromSecret("wandb-minio", "SECRET_KEY", false), conn.SecretKey) } func TestMigrateLegacyBucket_HostPortEndpointWithBucketAndPrefixInPath(t *testing.T) { @@ -540,8 +531,8 @@ func TestMigrateLegacyBucket_HostPortEndpointWithBucketAndPrefixInPath(t *testin require.Equal(t, []byte("team/project"), secret.Data["path"]) conn := wandb.Spec.ObjectStore[apiv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, "bucket", conn.Bucket.Key) - require.Equal(t, "path", conn.Path.Key) + require.Equal(t, "bucket", conn.Bucket.SecretKeyRef().Key) + require.Equal(t, "path", conn.Path.SecretKeyRef().Key) } func TestMigrateLegacyBucket_AWSBucketWithPathIsNotEndpoint(t *testing.T) { @@ -649,7 +640,7 @@ func TestMigrateLegacyBucket_PathPrefix(t *testing.T) { require.Equal(t, []byte("wandb-files"), secret.Data["path"]) conn := wandb.Spec.ObjectStore[apiv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, "path", conn.Path.Key) + require.Equal(t, "path", conn.Path.SecretKeyRef().Key) } func TestMigrateLegacyBucket_QueryOnlyPath(t *testing.T) { @@ -737,14 +728,8 @@ func TestMigrateLegacyBucket_PreSetCredentialsRespected(t *testing.T) { w.Spec.ObjectStore = map[string]apiv2.ObjectStoreSpec{ apiv2.DefaultInstanceName: { ExternalObjectStore: &apiv2.ObjectStoreConnection{ - AccessKey: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset"}, - Key: "ACCESS_KEY", - }, - SecretKey: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset"}, - Key: "SECRET_KEY", - }, + AccessKey: apiv2.ValueFromSecret("preset", "ACCESS_KEY", false), + SecretKey: apiv2.ValueFromSecret("preset", "SECRET_KEY", false), }, }, } @@ -760,9 +745,9 @@ func TestMigrateLegacyBucket_PreSetCredentialsRespected(t *testing.T) { require.Contains(t, secret.Data, "bucket") conn := wandb.Spec.ObjectStore[apiv2.DefaultInstanceName].ExternalObjectStore - require.Equal(t, "preset", conn.AccessKey.Name) - require.Equal(t, "preset", conn.SecretKey.Name) - require.Equal(t, "wandb-bucket-converted", conn.Bucket.Name) + require.Equal(t, "preset", conn.AccessKey.SecretKeyRef().Name) + require.Equal(t, "preset", conn.SecretKey.SecretKeyRef().Name) + require.Equal(t, "wandb-bucket-converted", conn.Bucket.SecretKeyRef().Name) } func TestMigrateLegacyBucket_UnknownFieldsIgnored(t *testing.T) { @@ -809,11 +794,11 @@ func TestMigrateLegacyOIDC_AllLiterals(t *testing.T) { require.Equal(t, []byte("https://idp.example.com"), secret.Data["issuerUrl"]) oidc := wandb.Spec.Wandb.OIDC - require.Equal(t, "wandb-oidc-converted", oidc.ClientId.Name) - require.Equal(t, "clientId", oidc.ClientId.Key) - require.Equal(t, "clientSecret", oidc.ClientSecret.Key) - require.Equal(t, "authMethod", oidc.AuthMethod.Key) - require.Equal(t, "issuerUrl", oidc.IssuerUrl.Key) + require.Equal(t, "wandb-oidc-converted", oidc.ClientId.SecretKeyRef().Name) + require.Equal(t, "clientId", oidc.ClientId.SecretKeyRef().Key) + require.Equal(t, "clientSecret", oidc.ClientSecret.SecretKeyRef().Key) + require.Equal(t, "authMethod", oidc.AuthMethod.SecretKeyRef().Key) + require.Equal(t, "issuerUrl", oidc.IssuerUrl.SecretKeyRef().Key) var fresh apiv2.WeightsAndBiases require.NoError(t, client.Get(context.Background(), types.NamespacedName{Name: "wandb", Namespace: "default"}, &fresh)) @@ -825,10 +810,7 @@ func TestMigrateLegacyOIDC_PreSetClientSecretRespected(t *testing.T) { client, wandb := newMigrationFixture(t, map[string]string{ apiv1.OIDCPendingAnnotation: payload, }, func(w *apiv2.WeightsAndBiases) { - w.Spec.Wandb.OIDC.ClientSecret = corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset-oidc"}, - Key: "PRESET", - } + w.Spec.Wandb.OIDC.ClientSecret = apiv2.ValueFromSecret("preset-oidc", "PRESET", false) }) _, err := migrateLegacyAnnotations(context.Background(), client, wandb) @@ -840,8 +822,8 @@ func TestMigrateLegacyOIDC_PreSetClientSecretRespected(t *testing.T) { require.NotContains(t, secret.Data, "clientSecret") oidc := wandb.Spec.Wandb.OIDC - require.Equal(t, "preset-oidc", oidc.ClientSecret.Name) - require.Equal(t, "PRESET", oidc.ClientSecret.Key) + require.Equal(t, "preset-oidc", oidc.ClientSecret.SecretKeyRef().Name) + require.Equal(t, "PRESET", oidc.ClientSecret.SecretKeyRef().Key) } func TestMigrateLegacyOIDC_MalformedJSON(t *testing.T) { @@ -878,14 +860,14 @@ func TestMigrateLegacyClickHouse_FullLiteralPayload(t *testing.T) { conn := fresh.Spec.ClickHouse[apiv2.DefaultInstanceName].ExternalClickHouse require.NotNil(t, conn) require.Nil(t, fresh.Spec.ClickHouse[apiv2.DefaultInstanceName].ManagedClickHouse) - require.Equal(t, "wandb-clickhouse-converted", conn.Host.Name) - require.Equal(t, "host", conn.Host.Key) - require.Equal(t, "httpPort", conn.HTTPPort.Key) - require.Equal(t, "database", conn.Database.Key) - require.Equal(t, "username", conn.Username.Key) - require.Equal(t, "password", conn.Password.Key) - require.Empty(t, conn.TCPPort.Name) - require.Empty(t, conn.URL.Name) + require.Equal(t, "wandb-clickhouse-converted", conn.Host.SecretKeyRef().Name) + require.Equal(t, "host", conn.Host.SecretKeyRef().Key) + require.Equal(t, "httpPort", conn.HTTPPort.SecretKeyRef().Key) + require.Equal(t, "database", conn.Database.SecretKeyRef().Key) + require.Equal(t, "username", conn.Username.SecretKeyRef().Key) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) + require.Nil(t, conn.TCPPort.SecretKeyRef()) + require.Nil(t, conn.URL.SecretKeyRef()) } func TestMigrateLegacyClickHouse_PartialPayload(t *testing.T) { @@ -908,11 +890,11 @@ func TestMigrateLegacyClickHouse_PartialPayload(t *testing.T) { conn := wandb.Spec.ClickHouse[apiv2.DefaultInstanceName].ExternalClickHouse require.NotNil(t, conn) - require.Equal(t, "host", conn.Host.Key) - require.Equal(t, "password", conn.Password.Key) - require.Empty(t, conn.HTTPPort.Name) - require.Empty(t, conn.Database.Name) - require.Empty(t, conn.Username.Name) + require.Equal(t, "host", conn.Host.SecretKeyRef().Key) + require.Equal(t, "password", conn.Password.SecretKeyRef().Key) + require.Nil(t, conn.HTTPPort.SecretKeyRef()) + require.Nil(t, conn.Database.SecretKeyRef()) + require.Nil(t, conn.Username.SecretKeyRef()) } // TestMigrateLegacyClickHouse_PreSetFieldsAreRespected: preset selectors are @@ -925,10 +907,7 @@ func TestMigrateLegacyClickHouse_PreSetFieldsAreRespected(t *testing.T) { w.Spec.ClickHouse = map[string]apiv2.ClickHouseSpec{ apiv2.DefaultInstanceName: { ExternalClickHouse: &apiv2.ClickHouseConnection{ - Password: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "preset-ch"}, - Key: "PRESET", - }, + Password: apiv2.ValueFromSecret("preset-ch", "PRESET", false), }, }, } @@ -943,8 +922,8 @@ func TestMigrateLegacyClickHouse_PreSetFieldsAreRespected(t *testing.T) { require.NotContains(t, secret.Data, "password", "preset password selector must be respected") conn := wandb.Spec.ClickHouse[apiv2.DefaultInstanceName].ExternalClickHouse - require.Equal(t, "preset-ch", conn.Password.Name) - require.Equal(t, "PRESET", conn.Password.Key) + require.Equal(t, "preset-ch", conn.Password.SecretKeyRef().Name) + require.Equal(t, "PRESET", conn.Password.SecretKeyRef().Key) } func TestMigrateLegacyClickHouse_PortStringified(t *testing.T) { diff --git a/internal/controller/reconciler/oidc_env_test.go b/internal/controller/reconciler/oidc_env_test.go index f73a440f..2b503878 100644 --- a/internal/controller/reconciler/oidc_env_test.go +++ b/internal/controller/reconciler/oidc_env_test.go @@ -8,7 +8,6 @@ import ( apiv2 "github.com/wandb/operator/api/v2" serverManifest "github.com/wandb/operator/pkg/wandb/manifest" - corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client/fake" ) @@ -18,10 +17,7 @@ func oidcTestCR() *apiv2.WeightsAndBiases { ObjectMeta: metav1.ObjectMeta{Name: "wandb", Namespace: "default"}, } wandb.Spec.Wandb.OIDC = apiv2.OidcSpec{ - ClientId: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: "my-oidc"}, - Key: "clientId", - }, + ClientId: apiv2.ValueFromSecret("my-oidc", "clientId", false), SessionLength: "48h", } return wandb diff --git a/internal/controller/reconciler/pods.go b/internal/controller/reconciler/pods.go index ab4e24b3..d1952339 100644 --- a/internal/controller/reconciler/pods.go +++ b/internal/controller/reconciler/pods.go @@ -215,7 +215,11 @@ func resolveEnvvars(ctx context.Context, client ctrlClient.Client, wandb *v2.Wei if !ok { continue } - selector := status.Connection.URL + ref := status.Connection.URL.SecretKeyRef() + if ref == nil { + continue + } + selector := *ref // Record for potential direct assignment case singleSecretSelector = selector secretOnlyCount++ @@ -225,7 +229,11 @@ func resolveEnvvars(ctx context.Context, client ctrlClient.Client, wandb *v2.Wei if !ok { continue } - selector := status.Connection.URL + ref := status.Connection.URL.SecretKeyRef() + if ref == nil { + continue + } + selector := *ref singleSecretSelector = selector secretOnlyCount++ addSecretComponent(selector, idx) @@ -234,8 +242,12 @@ func resolveEnvvars(ctx context.Context, client ctrlClient.Client, wandb *v2.Wei if !ok { continue } + urlRef := status.Connection.URL.SecretKeyRef() + if urlRef == nil { + continue + } selector := v1.SecretKeySelector{ - LocalObjectReference: status.Connection.URL.LocalObjectReference, + LocalObjectReference: urlRef.LocalObjectReference, } switch src.Field { case "host": @@ -262,8 +274,12 @@ func resolveEnvvars(ctx context.Context, client ctrlClient.Client, wandb *v2.Wei if !ok { continue } + urlRef := status.Connection.URL.SecretKeyRef() + if urlRef == nil { + continue + } selector := v1.SecretKeySelector{ - LocalObjectReference: status.Connection.URL.LocalObjectReference, + LocalObjectReference: urlRef.LocalObjectReference, } switch src.Field { case "host": @@ -289,15 +305,19 @@ func resolveEnvvars(ctx context.Context, client ctrlClient.Client, wandb *v2.Wei addSecretComponent(selector, idx) case "kafka": // kafka can be referenced as a full URL (no field) or by specific fields (host/port) + urlRef := wandb.Status.KafkaStatus.Connection.URL.SecretKeyRef() + if urlRef == nil { + continue + } if src.Field == "" { - selector := wandb.Status.KafkaStatus.Connection.URL + selector := *urlRef singleSecretSelector = selector secretOnlyCount++ addSecretComponent(selector, idx) break } selector := v1.SecretKeySelector{ - LocalObjectReference: wandb.Status.KafkaStatus.Connection.URL.LocalObjectReference, + LocalObjectReference: urlRef.LocalObjectReference, } switch src.Field { case "host": diff --git a/internal/controller/reconciler/pods_instance_test.go b/internal/controller/reconciler/pods_instance_test.go index 90ed00b0..2a7194c6 100644 --- a/internal/controller/reconciler/pods_instance_test.go +++ b/internal/controller/reconciler/pods_instance_test.go @@ -12,11 +12,8 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client/fake" ) -func mysqlURLSelector(secretName string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: secretName}, - Key: "url", - } +func mysqlURLSelector(secretName string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(secretName, "url", false) } func wandbWithTwoMysqlInstances() *apiv2.WeightsAndBiases { diff --git a/internal/controller/reconciler/proxy_env.go b/internal/controller/reconciler/proxy_env.go index 440132ac..277f640c 100644 --- a/internal/controller/reconciler/proxy_env.go +++ b/internal/controller/reconciler/proxy_env.go @@ -53,29 +53,28 @@ func joinNoProxy(entries []string) string { return strings.Join(out, ",") } -// proxyValueEnvVars turns one ProxyValue into the upper/lower env-var pair for -// the given base name. A literal value becomes a literal env var; a valueFrom +// proxyValueEnvVars turns one ValueOrSecret into the upper/lower env-var pair for +// the given base name. A literal value becomes a literal env var; a secret ref // becomes a SecretKeyRef env source (both casings reference the same key) so // credential-bearing URLs stay in the Secret and never land in the workload // spec. Returns nil when the value is unset. -func proxyValueEnvVars(upper, lower string, pv *apiv2.ProxyValue) []corev1.EnvVar { +func proxyValueEnvVars(upper, lower string, pv *apiv2.ValueOrSecret) []corev1.EnvVar { if pv == nil { return nil } - switch { - case pv.Value != "": + if pv.Value != "" { return []corev1.EnvVar{ {Name: upper, Value: pv.Value}, {Name: lower, Value: pv.Value}, } - case pv.ValueFrom != nil && pv.ValueFrom.SecretKeyRef != nil: + } + if ref := pv.SecretKeyRef(); ref != nil { return []corev1.EnvVar{ - {Name: upper, ValueFrom: &corev1.EnvVarSource{SecretKeyRef: pv.ValueFrom.SecretKeyRef.DeepCopy()}}, - {Name: lower, ValueFrom: &corev1.EnvVarSource{SecretKeyRef: pv.ValueFrom.SecretKeyRef.DeepCopy()}}, + {Name: upper, ValueFrom: &corev1.EnvVarSource{SecretKeyRef: ref.DeepCopy()}}, + {Name: lower, ValueFrom: &corev1.EnvVarSource{SecretKeyRef: ref.DeepCopy()}}, } - default: - return nil } + return nil } // proxyEnvVars builds the full six-variable proxy env set for spec.global.proxy: diff --git a/internal/controller/reconciler/proxy_env_test.go b/internal/controller/reconciler/proxy_env_test.go index 06d16e44..51fe93b1 100644 --- a/internal/controller/reconciler/proxy_env_test.go +++ b/internal/controller/reconciler/proxy_env_test.go @@ -62,8 +62,8 @@ func TestComputeNoProxyNoAPIServerHost(t *testing.T) { func TestProxyEnvVarsLiteral(t *testing.T) { t.Setenv("KUBERNETES_SERVICE_HOST", "10.96.0.1") proxy := &apiv2.ProxySpec{ - HTTPProxy: &apiv2.ProxyValue{Value: "http://proxy:3128"}, - HTTPSProxy: &apiv2.ProxyValue{Value: "http://proxy:3128"}, + HTTPProxy: &apiv2.ValueOrSecret{Value: "http://proxy:3128"}, + HTTPSProxy: &apiv2.ValueOrSecret{Value: "http://proxy:3128"}, NoProxy: []string{"wandb.localhost"}, } vars := proxyEnvVars(proxy) @@ -88,8 +88,8 @@ func TestProxyEnvVarsLiteral(t *testing.T) { func TestProxyEnvVarsValueFromStaysSecretRef(t *testing.T) { proxy := &apiv2.ProxySpec{ - HTTPSProxy: &apiv2.ProxyValue{ - ValueFrom: &apiv2.ProxyValueSource{ + HTTPSProxy: &apiv2.ValueOrSecret{ + ValueFrom: &apiv2.SecretValueSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{Name: "egress-proxy"}, Key: "httpsProxy", @@ -143,7 +143,7 @@ func TestApplyProxyToWorkload(t *testing.T) { // With proxy: appends missing vars, does not clobber an existing HTTP_PROXY // (appendMissing semantics — legacy/manifest precedence handled elsewhere). wandb := &apiv2.WeightsAndBiases{} - wandb.Spec.Global.Proxy = &apiv2.ProxySpec{HTTPProxy: &apiv2.ProxyValue{Value: "http://proxy:3128"}} + wandb.Spec.Global.Proxy = &apiv2.ProxySpec{HTTPProxy: &apiv2.ValueOrSecret{Value: "http://proxy:3128"}} got := applyProxyToWorkload(wandb, base) if v, _ := envByName(got, "HTTP_PROXY"); v.Value != "manifest-value" { t.Errorf("existing HTTP_PROXY should be preserved by appendMissing, got %q", v.Value) diff --git a/internal/controller/reconciler/reconcile_v2.go b/internal/controller/reconciler/reconcile_v2.go index 545c49bf..d1b6547e 100644 --- a/internal/controller/reconciler/reconcile_v2.go +++ b/internal/controller/reconciler/reconcile_v2.go @@ -1340,30 +1340,44 @@ func resolveCRFieldEnvValue(obj any, path string) (string, bool) { return value, true case bool: return strconv.FormatBool(value), true + case map[string]any: + // A ValueOrSecret envelope carrying a literal value. + tb, err := json.Marshal(value) + if err != nil { + return "", false + } + var v apiv2.ValueOrSecret + if err := json.Unmarshal(tb, &v); err == nil && v.Value != "" { + return v.Value, true + } + return "", false default: return "", false } } +// resolveCRFieldSecretSelector resolves a dotted CR field into the effective +// secret selector. It understands the ValueOrSecret envelope (valueFrom or the +// legacy {name, key} shape) as well as a bare SecretKeySelector node. A literal +// value yields no selector (the caller falls back to resolveCRFieldEnvValue). func resolveCRFieldSecretSelector(obj any, path string) (corev1.SecretKeySelector, bool) { cur, ok := resolveCRField(obj, path) if !ok { return corev1.SecretKeySelector{}, false } - // Re-marshal the terminal node into a SecretKeySelector so we honor the same - // json tags (name/key/optional) the CRD uses. tb, err := json.Marshal(cur) if err != nil { return corev1.SecretKeySelector{}, false } - var sel corev1.SecretKeySelector - if err := json.Unmarshal(tb, &sel); err != nil { + var v apiv2.ValueOrSecret + if err := json.Unmarshal(tb, &v); err != nil { return corev1.SecretKeySelector{}, false } - if sel.Name == "" || sel.Key == "" { + ref := v.SecretKeyRef() + if ref == nil || ref.Name == "" || ref.Key == "" { return corev1.SecretKeySelector{}, false } - return sel, true + return *ref, true } // allInstancesReady reports whether every instance (managed or external) has a diff --git a/internal/controller/weightsandbiases_controller_networking_test.go b/internal/controller/weightsandbiases_controller_networking_test.go index ab18c5ed..b7620c79 100644 --- a/internal/controller/weightsandbiases_controller_networking_test.go +++ b/internal/controller/weightsandbiases_controller_networking_test.go @@ -296,10 +296,7 @@ func markWandbReadyForNetworking(ctx context.Context, name, namespace string) *a apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, Connection: apiv2.MysqlConnection{ - URL: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: "mysql-url", - }, + URL: apiv2.ValueFromSecret(name, "mysql-url", false), }, }, } @@ -318,10 +315,7 @@ func markWandbReadyForNetworking(ctx context.Context, name, namespace string) *a apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, Connection: apiv2.ClickHouseConnection{ - URL: corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: "clickhouse-url", - }, + URL: apiv2.ValueFromSecret(name, "clickhouse-url", false), }, }, } diff --git a/internal/controller/weightsandbiases_controller_test.go b/internal/controller/weightsandbiases_controller_test.go index aec38f4f..a4d0a419 100644 --- a/internal/controller/weightsandbiases_controller_test.go +++ b/internal/controller/weightsandbiases_controller_test.go @@ -225,14 +225,14 @@ var _ = Describe("WeightsAndBiases Controller V2", func() { wandb.Status.MySQLStatus = map[string]apiv2.MysqlInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.MysqlConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.MysqlConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} wandb.Status.RedisStatus = map[string]apiv2.RedisInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.KafkaStatus.Ready = true wandb.Status.ObjectStoreStatus = map[string]apiv2.ObjectStoreInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.ClickHouseStatus = map[string]apiv2.ClickHouseInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.ClickHouseConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.ClickHouseConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} Expect(k8sClient.Status().Update(ctx, wandb)).Should(Succeed()) @@ -310,14 +310,14 @@ var _ = Describe("WeightsAndBiases Controller V2", func() { wandb.Status.MySQLStatus = map[string]apiv2.MysqlInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.MysqlConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.MysqlConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} wandb.Status.RedisStatus = map[string]apiv2.RedisInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.KafkaStatus.Ready = true wandb.Status.ObjectStoreStatus = map[string]apiv2.ObjectStoreInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.ClickHouseStatus = map[string]apiv2.ClickHouseInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.ClickHouseConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.ClickHouseConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} Expect(k8sClient.Status().Update(ctx, wandb)).Should(Succeed()) @@ -398,10 +398,10 @@ var _ = Describe("WeightsAndBiases Controller V2", func() { wandb.Status.ObjectStoreStatus = map[string]apiv2.ObjectStoreInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.ClickHouseStatus = map[string]apiv2.ClickHouseInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} mysqlStatus := wandb.Status.MySQLStatus[apiv2.DefaultInstanceName] - mysqlStatus.Connection.URL = v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"} + mysqlStatus.Connection.URL = apiv2.ValueFromSecret(WandbName, "test", false) wandb.Status.MySQLStatus[apiv2.DefaultInstanceName] = mysqlStatus clickHouseStatus := wandb.Status.ClickHouseStatus[apiv2.DefaultInstanceName] - clickHouseStatus.Connection.URL = v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"} + clickHouseStatus.Connection.URL = apiv2.ValueFromSecret(WandbName, "test", false) wandb.Status.ClickHouseStatus[apiv2.DefaultInstanceName] = clickHouseStatus wandb.Status.Wandb.Migration.Version = wandb.Spec.Wandb.Version wandb.Status.Wandb.Migration.LastSuccessVersion = wandb.Spec.Wandb.Version @@ -502,14 +502,14 @@ var _ = Describe("WeightsAndBiases Controller V2", func() { Expect(k8sClient.Get(ctx, wandbLookupKey, wandb)).Should(Succeed()) wandb.Status.MySQLStatus = map[string]apiv2.MysqlInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.MysqlConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.MysqlConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} wandb.Status.RedisStatus = map[string]apiv2.RedisInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.KafkaStatus.Ready = true wandb.Status.ObjectStoreStatus = map[string]apiv2.ObjectStoreInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.ClickHouseStatus = map[string]apiv2.ClickHouseInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.ClickHouseConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.ClickHouseConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} wandb.Status.Wandb.Migration.Version = wandb.Spec.Wandb.Version wandb.Status.Wandb.Migration.LastSuccessVersion = wandb.Spec.Wandb.Version @@ -634,10 +634,10 @@ var _ = Describe("WeightsAndBiases Controller V2", func() { wandb.Status.Wandb.Migration.Ready = false wandb.Status.Wandb.Migration.Reason = "Running" mysqlStatus := wandb.Status.MySQLStatus[apiv2.DefaultInstanceName] - mysqlStatus.Connection.URL = v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"} + mysqlStatus.Connection.URL = apiv2.ValueFromSecret(WandbName, "test", false) wandb.Status.MySQLStatus[apiv2.DefaultInstanceName] = mysqlStatus clickHouseStatus := wandb.Status.ClickHouseStatus[apiv2.DefaultInstanceName] - clickHouseStatus.Connection.URL = v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"} + clickHouseStatus.Connection.URL = apiv2.ValueFromSecret(WandbName, "test", false) wandb.Status.ClickHouseStatus[apiv2.DefaultInstanceName] = clickHouseStatus Expect(k8sClient.Status().Update(ctx, wandb)).Should(Succeed()) @@ -701,14 +701,14 @@ var _ = Describe("WeightsAndBiases Controller V2", func() { // Mark infra as ready and migration as complete for old version wandb.Status.MySQLStatus = map[string]apiv2.MysqlInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.MysqlConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.MysqlConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} wandb.Status.RedisStatus = map[string]apiv2.RedisInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.KafkaStatus.Ready = true wandb.Status.ObjectStoreStatus = map[string]apiv2.ObjectStoreInfraStatus{apiv2.DefaultInstanceName: {WBInfraStatus: apiv2.WBInfraStatus{Ready: true}}} wandb.Status.ClickHouseStatus = map[string]apiv2.ClickHouseInfraStatus{apiv2.DefaultInstanceName: { WBInfraStatus: apiv2.WBInfraStatus{Ready: true}, - Connection: apiv2.ClickHouseConnection{URL: v1.SecretKeySelector{LocalObjectReference: v1.LocalObjectReference{Name: WandbName}, Key: "test"}}, + Connection: apiv2.ClickHouseConnection{URL: apiv2.ValueFromSecret(WandbName, "test", false)}, }} wandb.Status.Wandb.Migration.Version = oldVersion wandb.Status.Wandb.Migration.LastSuccessVersion = oldVersion diff --git a/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml b/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml index 921d192e..39a497e6 100644 --- a/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml +++ b/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml @@ -530,92 +530,190 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic httpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tcpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedClickhouse: properties: @@ -1225,6 +1323,12 @@ spec: properties: httpProxy: properties: + key: + type: string + name: + type: string + optional: + type: boolean value: type: string valueFrom: @@ -1246,6 +1350,12 @@ spec: type: object httpsProxy: properties: + key: + type: string + name: + type: string + optional: + type: boolean value: type: string valueFrom: @@ -1834,131 +1944,271 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCert: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedMysql: properties: @@ -2583,144 +2833,298 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic bucket: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic - endpoint: - properties: - key: - type: string - name: - default: "" + value: type: string - optional: - type: boolean - required: - - key + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object + endpoint: + properties: + key: + type: string + name: + type: string + optional: + type: boolean + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic forcePathStyle: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic path: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic provider: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic region: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic secretKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tlsEnabled: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedObjectStore: properties: @@ -3269,79 +3673,163 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object managedRedis: properties: @@ -4095,53 +4583,109 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clientId: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clientSecret: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object issuerUrl: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sessionLength: type: string type: object @@ -4465,92 +5009,190 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic httpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tcpPort: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -4714,66 +5356,136 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clusterID: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -4828,131 +5540,271 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic host: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic - password: - properties: - key: - type: string - name: - default: "" + value: type: string - optional: - type: boolean - required: - - key - type: object - x-kubernetes-map-type: atomic + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object + password: + properties: + key: + type: string + name: + type: string + optional: + type: boolean + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object + type: object port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCert: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -5008,144 +5860,298 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic bucket: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic endpoint: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic forcePathStyle: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic path: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic provider: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic region: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic secretKey: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tlsEnabled: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean @@ -5206,79 +6212,163 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic sslCa: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic tls: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic url: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object ready: type: boolean diff --git a/internal/logx/handler.go b/internal/logx/handler.go index 68d06a59..d3aa4709 100644 --- a/internal/logx/handler.go +++ b/internal/logx/handler.go @@ -10,14 +10,19 @@ const LoggerKey = "LOGGER" func NewHandler(opts *Options, loggerName string) *Handler { opts = withDefaults(opts) + // Layer secret redaction onto ReplaceAttr in a local copy so repeated + // NewHandler calls never wrap the shared opts more than once. + ho := *opts.HandlerOptions + ho.ReplaceAttr = chainReplaceAttr(opts.HandlerOptions.ReplaceAttr, redact) + var baseHandler slog.Handler switch opts.Format { case JsonFormat: - baseHandler = slog.NewJSONHandler(opts.Output, opts.HandlerOptions) + baseHandler = slog.NewJSONHandler(opts.Output, &ho) case PrettyFormat: - baseHandler = BuildPrettyHandler(opts) + baseHandler = BuildPrettyHandler(opts, redact) default: - baseHandler = slog.NewTextHandler(opts.Output, opts.HandlerOptions) + baseHandler = slog.NewTextHandler(opts.Output, &ho) } defaultLevel := slog.LevelInfo diff --git a/internal/logx/pretty.go b/internal/logx/pretty.go index 95fd77db..f6c4f581 100644 --- a/internal/logx/pretty.go +++ b/internal/logx/pretty.go @@ -8,14 +8,17 @@ import ( "github.com/lmittmann/tint" ) -func BuildPrettyHandler(opts *Options) slog.Handler { +func BuildPrettyHandler(opts *Options, extra func([]string, slog.Attr) slog.Attr) slog.Handler { return tint.NewHandler(opts.Output, &tint.Options{ TimeFormat: time.TimeOnly + ".000000", - ReplaceAttr: func(groups []string, a slog.Attr) slog.Attr { - if strings.EqualFold(a.Key, LoggerKey) { - return slog.String(strings.ToUpper(a.Key), a.Value.String()) - } - return a - }, + ReplaceAttr: chainReplaceAttr( + func(groups []string, a slog.Attr) slog.Attr { + if strings.EqualFold(a.Key, LoggerKey) { + return slog.String(strings.ToUpper(a.Key), a.Value.String()) + } + return a + }, + extra, + ), }) } diff --git a/internal/logx/redact.go b/internal/logx/redact.go new file mode 100644 index 00000000..e80d750e --- /dev/null +++ b/internal/logx/redact.go @@ -0,0 +1,26 @@ +package logx + +import ( + "log/slog" + + "github.com/m-mizutani/masq" +) + +// redact masks any struct field tagged `masq:"secret"` in a logged attribute, +// so sensitive connection values never reach the operator log. It is applied to +// every handler format via chainReplaceAttr. +var redact = masq.New(masq.WithTag("secret")) + +// chainReplaceAttr composes slog ReplaceAttr functions left to right, skipping +// nil entries. It lets us layer masq redaction on top of a handler's existing +// ReplaceAttr without either clobbering the other. +func chainReplaceAttr(fns ...func([]string, slog.Attr) slog.Attr) func([]string, slog.Attr) slog.Attr { + return func(groups []string, a slog.Attr) slog.Attr { + for _, fn := range fns { + if fn != nil { + a = fn(groups, a) + } + } + return a + } +} diff --git a/internal/logx/redact_test.go b/internal/logx/redact_test.go new file mode 100644 index 00000000..6e44abcd --- /dev/null +++ b/internal/logx/redact_test.go @@ -0,0 +1,33 @@ +package logx + +import ( + "bytes" + "log/slog" + "strings" + "testing" +) + +// TestRedactsSecretTaggedFields verifies that a struct field tagged +// `masq:"secret"` is redacted in the log output while non-secret fields survive. +func TestRedactsSecretTaggedFields(t *testing.T) { + type conn struct { + Host string + Password string `masq:"secret"` + } + + var buf bytes.Buffer + h := NewHandler(&Options{ + HandlerOptions: &slog.HandlerOptions{}, + Output: &buf, + Format: JsonFormat, + }, "") + slog.New(h).Info("connection", slog.Any("conn", conn{Host: "db.example.com", Password: "sup3rs3cret"})) + + out := buf.String() + if strings.Contains(out, "sup3rs3cret") { + t.Fatalf("secret leaked into log output: %s", out) + } + if !strings.Contains(out, "db.example.com") { + t.Fatalf("non-secret value should be present: %s", out) + } +} diff --git a/internal/webhook/v2/weightsandbiases_proxy_test.go b/internal/webhook/v2/weightsandbiases_proxy_test.go index 8a7f4162..b0f88375 100644 --- a/internal/webhook/v2/weightsandbiases_proxy_test.go +++ b/internal/webhook/v2/weightsandbiases_proxy_test.go @@ -15,7 +15,7 @@ func wandbWithProxy(proxy *appsv2.ProxySpec) *appsv2.WeightsAndBiases { } func TestValidateProxySpec(t *testing.T) { - secretRef := &appsv2.ProxyValueSource{ + secretRef := &appsv2.SecretValueSource{ SecretKeyRef: &corev1.SecretKeySelector{ LocalObjectReference: corev1.LocalObjectReference{Name: "egress-proxy"}, Key: "httpsProxy", @@ -27,15 +27,15 @@ func TestValidateProxySpec(t *testing.T) { wantErr string // substring; "" = accept }{ {"nil proxy", nil, ""}, - {"literal http url", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "http://proxy.corp:3128"}}, ""}, - {"secret-backed https", &appsv2.ProxySpec{HTTPSProxy: &appsv2.ProxyValue{ValueFrom: secretRef}}, ""}, - {"noProxy extras ok", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "http://p:3128"}, NoProxy: []string{"internal.example.com", "10.0.0.0/8"}}, ""}, - {"both value and valueFrom", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "http://p:3128", ValueFrom: secretRef}}, "exactly one"}, - {"neither value nor valueFrom", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{}}, "one of value or valueFrom is required"}, - {"userinfo in literal", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "http://user:pass@proxy:3128"}}, "must not contain credentials"}, - {"bad scheme", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "socks5://proxy:1080"}}, "scheme must be http or https"}, - {"comma in noProxy", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "http://p:3128"}, NoProxy: []string{"a,b"}}, "must not contain commas"}, - {"empty noProxy entry", &appsv2.ProxySpec{HTTPProxy: &appsv2.ProxyValue{Value: "http://p:3128"}, NoProxy: []string{""}}, "must not be empty"}, + {"literal http url", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "http://proxy.corp:3128"}}, ""}, + {"secret-backed https", &appsv2.ProxySpec{HTTPSProxy: &appsv2.ValueOrSecret{ValueFrom: secretRef}}, ""}, + {"noProxy extras ok", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "http://p:3128"}, NoProxy: []string{"internal.example.com", "10.0.0.0/8"}}, ""}, + {"both value and valueFrom", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "http://p:3128", ValueFrom: secretRef}}, "exactly one"}, + {"neither value nor valueFrom", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{}}, "one of value or valueFrom is required"}, + {"userinfo in literal", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "http://user:pass@proxy:3128"}}, "must not contain credentials"}, + {"bad scheme", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "socks5://proxy:1080"}}, "scheme must be http or https"}, + {"comma in noProxy", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "http://p:3128"}, NoProxy: []string{"a,b"}}, "must not contain commas"}, + {"empty noProxy entry", &appsv2.ProxySpec{HTTPProxy: &appsv2.ValueOrSecret{Value: "http://p:3128"}, NoProxy: []string{""}}, "must not be empty"}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/webhook/v2/weightsandbiases_webhook.go b/internal/webhook/v2/weightsandbiases_webhook.go index 6a3d62ff..78b8b934 100644 --- a/internal/webhook/v2/weightsandbiases_webhook.go +++ b/internal/webhook/v2/weightsandbiases_webhook.go @@ -135,6 +135,8 @@ func (d *WeightsAndBiasesCustomDefaulter) Default(ctx context.Context, obj runti applyClickHouseDefaults(wandb) applyProbeDefaults(wandb) + normalizeConnections(wandb) + if defaultStore, ok := wandb.Spec.ObjectStore["default"]; ok && defaultStore.ManagedObjectStore != nil { wandb.Spec.Wandb.BucketProxy = true } @@ -494,6 +496,7 @@ func validateMySQLSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { "managedMysql and externalMysql are mutually exclusive", )) } + errors = append(errors, validateMysqlConnection(spec.ExternalMysql, instancePath.Child("externalMysql"))...) if managed := spec.ManagedMysql; managed != nil { if managed.Replicas != 0 && !appsv2.ValidMysqlReplicaCount(managed.Replicas) { errors = append(errors, field.Invalid( @@ -525,10 +528,13 @@ func validateRedisSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { )) } - if externalRedis := spec.ExternalRedis; externalRedis != nil && !hasPendingLegacyRedis { + if externalRedis := spec.ExternalRedis; externalRedis != nil { externalPath := instancePath.Child("externalRedis") - errors = append(errors, validateRequiredSecretSelector(externalRedis.Host, externalPath.Child("host"))...) - errors = append(errors, validateRequiredSecretSelector(externalRedis.Port, externalPath.Child("port"))...) + errors = append(errors, validateRedisConnection(externalRedis, externalPath)...) + if !hasPendingLegacyRedis { + errors = append(errors, validateRequiredValueOrSecret(externalRedis.Host, externalPath.Child("host"))...) + errors = append(errors, validateRequiredValueOrSecret(externalRedis.Port, externalPath.Child("port"))...) + } } if spec.ManagedRedis == nil { @@ -549,13 +555,129 @@ func validateRedisSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { return errors } -func validateRequiredSecretSelector(selector corev1.SecretKeySelector, path *field.Path) field.ErrorList { +// validateRequiredValueOrSecret errors when the value is unset. Exclusivity is +// validated separately by the per-connection validators. +func validateRequiredValueOrSecret(v appsv2.ValueOrSecret, path *field.Path) field.ErrorList { + if v.IsZero() { + return field.ErrorList{field.Required(path, "a value or secret reference is required")} + } + return nil +} + +// validateValueOrSecret enforces that a ValueOrSecret sets at most one of a +// literal value or a secret reference, and that a ValueFrom carries a +// secretKeyRef. It does not require a value; callers enforce requiredness. The +// legacy name/key shape counts as the secret arm. The rejected value is redacted +// so a secret literal is never echoed into the admission response. +func validateValueOrSecret(v appsv2.ValueOrSecret, path *field.Path) field.ErrorList { var errors field.ErrorList - if selector.Name == "" { - errors = append(errors, field.Required(path.Child("name"), "secret name is required")) + hasValue := v.Value != "" + hasSecret := v.SecretKeyRef() != nil + if hasValue && hasSecret { + errors = append(errors, field.Invalid(path, "[redacted]", "set exactly one of value or valueFrom, not both")) + return errors + } + if v.ValueFrom != nil && v.ValueFrom.SecretKeyRef == nil { + errors = append(errors, field.Required(path.Child("valueFrom").Child("secretKeyRef"), "valueFrom requires secretKeyRef")) } - if selector.Key == "" { - errors = append(errors, field.Required(path.Child("key"), "secret key is required")) + return errors +} + +// validateMysqlConnection checks value-or-secret exclusivity on each external +// MySQL field. Field order is fixed for deterministic errors. +func validateMysqlConnection(ext *appsv2.MysqlConnection, path *field.Path) field.ErrorList { + if ext == nil { + return nil + } + var errors field.ErrorList + for _, f := range []struct { + name string + val appsv2.ValueOrSecret + }{ + {"host", ext.Host}, {"port", ext.Port}, {"database", ext.Database}, + {"username", ext.Username}, {"password", ext.Password}, {"tls", ext.Tls}, + {"sslCa", ext.SslCa}, {"sslCert", ext.SslCert}, {"sslKey", ext.SslKey}, + } { + errors = append(errors, validateValueOrSecret(f.val, path.Child(f.name))...) + } + return errors +} + +// validateRedisConnection checks value-or-secret exclusivity on each external +// Redis field. +func validateRedisConnection(ext *appsv2.RedisConnection, path *field.Path) field.ErrorList { + if ext == nil { + return nil + } + var errors field.ErrorList + for _, f := range []struct { + name string + val appsv2.ValueOrSecret + }{ + {"host", ext.Host}, {"port", ext.Port}, {"password", ext.Password}, + {"tls", ext.Tls}, {"sslCa", ext.SslCa}, + } { + errors = append(errors, validateValueOrSecret(f.val, path.Child(f.name))...) + } + return errors +} + +// validateClickHouseConnection checks value-or-secret exclusivity on each +// external ClickHouse field. +func validateClickHouseConnection(ext *appsv2.ClickHouseConnection, path *field.Path) field.ErrorList { + if ext == nil { + return nil + } + var errors field.ErrorList + for _, f := range []struct { + name string + val appsv2.ValueOrSecret + }{ + {"host", ext.Host}, {"tcpPort", ext.TCPPort}, {"httpPort", ext.HTTPPort}, + {"database", ext.Database}, {"username", ext.Username}, {"password", ext.Password}, + } { + errors = append(errors, validateValueOrSecret(f.val, path.Child(f.name))...) + } + return errors +} + +// normalizeConnections rewrites the deprecated legacy {name, key} shape into the +// ValueFrom envelope on every external connection field and OIDC field, so +// stored objects converge on the envelope and existing CRs keep working without +// user action. Each Normalize() is nil-safe. +func normalizeConnections(wandb *appsv2.WeightsAndBiases) { + for _, spec := range wandb.Spec.MySQL { + spec.ExternalMysql.Normalize() + } + for _, spec := range wandb.Spec.Redis { + spec.ExternalRedis.Normalize() + } + for _, spec := range wandb.Spec.ClickHouse { + spec.ExternalClickHouse.Normalize() + } + for _, spec := range wandb.Spec.ObjectStore { + spec.ExternalObjectStore.Normalize() + } + wandb.Spec.Wandb.OIDC.Normalize() +} + +// validateObjectStoreConnection checks value-or-secret exclusivity on each +// external object-store field. Field order is fixed for deterministic errors. +func validateObjectStoreConnection(ext *appsv2.ObjectStoreConnection, path *field.Path) field.ErrorList { + if ext == nil { + return nil + } + var errors field.ErrorList + for _, f := range []struct { + name string + val appsv2.ValueOrSecret + }{ + {"provider", ext.Provider}, {"endpoint", ext.Endpoint}, {"port", ext.Port}, + {"accessKey", ext.AccessKey}, {"secretKey", ext.SecretKey}, {"bucket", ext.Bucket}, + {"path", ext.Path}, {"region", ext.Region}, {"tlsEnabled", ext.TlsEnabled}, + {"forcePathStyle", ext.ForcePathStyle}, + } { + errors = append(errors, validateValueOrSecret(f.val, path.Child(f.name))...) } return errors } @@ -610,13 +732,14 @@ func validateObjectStoreSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { if ext := spec.ExternalObjectStore; ext != nil { extPath := objectStorePath.Key(key).Child("externalObjectStore") - // provider is sourced from a secret key, so it is resolved and defaulted at reconcile time, not here. - if _, ok := wandb.GetAnnotations()[v1.BucketPendingAnnotation]; !ok && ext.Bucket.Name == "" { + // provider is resolved and defaulted at reconcile time, not here. + if _, ok := wandb.GetAnnotations()[v1.BucketPendingAnnotation]; !ok && ext.Bucket.IsZero() { errors = append(errors, field.Required( extPath.Child("bucket"), - "externalObjectStore requires a bucket secret reference", + "externalObjectStore requires a bucket value or secret reference", )) } + errors = append(errors, validateObjectStoreConnection(ext, extPath)...) } } @@ -638,6 +761,7 @@ func validateClickHouseSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { "managedClickhouse and externalClickhouse are mutually exclusive", )) } + errors = append(errors, validateClickHouseConnection(spec.ExternalClickHouse, instancePath.Child("externalClickhouse"))...) managed := spec.ManagedClickHouse if managed == nil { @@ -866,7 +990,7 @@ func validateProxySpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { proxy := wandb.Spec.Global.Proxy base := field.NewPath("spec").Child("global").Child("proxy") - validateValue := func(pv *appsv2.ProxyValue, child string) { + validateValue := func(pv *appsv2.ValueOrSecret, child string) { if pv == nil { return } diff --git a/internal/webhook/v2/weightsandbiases_webhook_test.go b/internal/webhook/v2/weightsandbiases_webhook_test.go index bd575d27..97d84cff 100644 --- a/internal/webhook/v2/weightsandbiases_webhook_test.go +++ b/internal/webhook/v2/weightsandbiases_webhook_test.go @@ -141,10 +141,9 @@ var _ = Describe("WeightsAndBiases Webhook", func() { _, err := validator.ValidateCreate(ctx, obj) Expect(err).To(HaveOccurred()) - Expect(err.Error()).To(ContainSubstring("externalRedis.host.name")) - Expect(err.Error()).To(ContainSubstring("externalRedis.host.key")) - Expect(err.Error()).To(ContainSubstring("externalRedis.port.name")) - Expect(err.Error()).To(ContainSubstring("externalRedis.port.key")) + Expect(err.Error()).To(ContainSubstring("externalRedis.host")) + Expect(err.Error()).To(ContainSubstring("externalRedis.port")) + Expect(err.Error()).To(ContainSubstring("a value or secret reference is required")) }) It("allows external Redis with host and port selectors", func() { @@ -467,9 +466,6 @@ func boolPtr(v bool) *bool { return &v } -func secretKeySelector(name, key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } +func secretKeySelector(name, key string) appsv2.ValueOrSecret { + return appsv2.ValueFromSecret(name, key, false) } diff --git a/pkg/utils/connection_secrets.go b/pkg/utils/connection_secrets.go index 00911cb6..06adaef1 100644 --- a/pkg/utils/connection_secrets.go +++ b/pkg/utils/connection_secrets.go @@ -5,6 +5,7 @@ import ( "fmt" "strings" + apiv2 "github.com/wandb/operator/api/v2" "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" @@ -36,3 +37,15 @@ func (r *ConnSecretResolver) Value(ctx context.Context, sel v1.SecretKeySelector } return strings.TrimSpace(string(secret.Data[sel.Key])), nil } + +// ValueOrSecret returns a ValueOrSecret's literal value, or the trimmed value the +// secret arm points at, or "" when unset. +func (r *ConnSecretResolver) ValueOrSecret(ctx context.Context, v apiv2.ValueOrSecret) (string, error) { + if v.Value != "" { + return strings.TrimSpace(v.Value), nil + } + if ref := v.SecretKeyRef(); ref != nil { + return r.Value(ctx, *ref) + } + return "", nil +} From 9ee9e8edf709ee866086e23f49f83db67e99feae Mon Sep 17 00:00:00 2001 From: Daniel Panzella Date: Thu, 20 Aug 2026 11:45:07 -0700 Subject: [PATCH 2/3] feat(api): Accept value or secret reference for notification config fields Extend the ValueOrSecret envelope to spec.wandb.notifications: email sink, SMTP host/port/username/password, and Slack clientId/clientSecret now accept a literal value or a secret reference (these arrived on main as plain SecretKeySelector). Adds Normalize() for the notification types (wired into the defaulter), per-field validation via validateValueOrSecret, and resolves SMTP + literal email sinks through ResolveValue. status.emailSink stays a plain SecretKeySelector (operator-written output). Secret-bearing fields carry a masq:"secret" tag. Co-Authored-By: Claude Opus 4.8 --- api/v2/weightsandbiases_types.go | 55 ++++++- api/v2/zz_generated.deepcopy.go | 2 +- .../apps.wandb.com_weightsandbiases.yaml | 154 ++++++++++++++---- .../secret_or_value_connection_fields.md | 9 +- internal/controller/reconciler/email.go | 44 +++-- internal/controller/reconciler/email_test.go | 20 ++- .../controller/reconciler/slack_env_test.go | 4 +- .../apps.wandb.com_weightsandbiases.yaml | 154 ++++++++++++++---- .../v2/weightsandbiases_notifications_test.go | 14 +- .../webhook/v2/weightsandbiases_webhook.go | 39 ++--- 10 files changed, 375 insertions(+), 120 deletions(-) diff --git a/api/v2/weightsandbiases_types.go b/api/v2/weightsandbiases_types.go index cc8f6ccc..5014dc2e 100644 --- a/api/v2/weightsandbiases_types.go +++ b/api/v2/weightsandbiases_types.go @@ -639,21 +639,60 @@ type NotificationsSpec struct { Slack *SlackSpec `json:"slack,omitempty"` } +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (n *NotificationsSpec) Normalize() { + if n == nil { + return + } + n.Email.Normalize() + n.Slack.Normalize() +} + type EmailSMTPSpec struct { - Host corev1.SecretKeySelector `json:"host"` - Port corev1.SecretKeySelector `json:"port"` - Username corev1.SecretKeySelector `json:"username"` - Password corev1.SecretKeySelector `json:"password"` + Host ValueOrSecret `json:"host"` + Port ValueOrSecret `json:"port"` + Username ValueOrSecret `json:"username"` + Password ValueOrSecret `json:"password" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (s *EmailSMTPSpec) Normalize() { + if s == nil { + return + } + s.Host.Normalize() + s.Port.Normalize() + s.Username.Normalize() + s.Password.Normalize() } type EmailSpec struct { - Sink *corev1.SecretKeySelector `json:"sink,omitempty"` - SMTP *EmailSMTPSpec `json:"smtp,omitempty"` + // Sink is a full notification sink URL; it may embed credentials. + Sink *ValueOrSecret `json:"sink,omitempty" masq:"secret"` + SMTP *EmailSMTPSpec `json:"smtp,omitempty"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (e *EmailSpec) Normalize() { + if e == nil { + return + } + e.Sink.Normalize() + e.SMTP.Normalize() } type SlackSpec struct { - ClientID corev1.SecretKeySelector `json:"clientId,omitempty"` - ClientSecret corev1.SecretKeySelector `json:"clientSecret,omitempty"` + ClientID ValueOrSecret `json:"clientId,omitempty"` + ClientSecret ValueOrSecret `json:"clientSecret,omitempty" masq:"secret"` +} + +// Normalize rewrites any legacy {name, key} field into the ValueFrom envelope. +func (s *SlackSpec) Normalize() { + if s == nil { + return + } + s.ClientID.Normalize() + s.ClientSecret.Normalize() } type ManagedInfraSpec struct { diff --git a/api/v2/zz_generated.deepcopy.go b/api/v2/zz_generated.deepcopy.go index f39d04ab..7f176b74 100644 --- a/api/v2/zz_generated.deepcopy.go +++ b/api/v2/zz_generated.deepcopy.go @@ -405,7 +405,7 @@ func (in *EmailSpec) DeepCopyInto(out *EmailSpec) { *out = *in if in.Sink != nil { in, out := &in.Sink, &out.Sink - *out = new(v1.SecretKeySelector) + *out = new(ValueOrSecret) (*in).DeepCopyInto(*out) } if in.SMTP != nil { diff --git a/config/crd/bases/apps.wandb.com_weightsandbiases.yaml b/config/crd/bases/apps.wandb.com_weightsandbiases.yaml index f9c9db06..e57b62e7 100644 --- a/config/crd/bases/apps.wandb.com_weightsandbiases.yaml +++ b/config/crd/bases/apps.wandb.com_weightsandbiases.yaml @@ -4608,14 +4608,28 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic smtp: properties: host: @@ -4623,53 +4637,109 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic required: - host - password @@ -4684,27 +4754,55 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clientSecret: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object type: object oidc: diff --git a/docs/design/wandb_v2/secret_or_value_connection_fields.md b/docs/design/wandb_v2/secret_or_value_connection_fields.md index 0fa1414e..cfa31c36 100644 --- a/docs/design/wandb_v2/secret_or_value_connection_fields.md +++ b/docs/design/wandb_v2/secret_or_value_connection_fields.md @@ -1,7 +1,7 @@ # String-or-secret connection fields **Status:** Implemented (Option 2) — validated by `make lint`/`make test` and westest -**Scope:** `api/v2` external-connection + OIDC fields +**Scope:** `api/v2` external-connection, OIDC, and notification (email/Slack) fields **Target release:** during v2 beta (`2.0.0-beta.3` today), before v2 GA ## Implementation status @@ -19,6 +19,13 @@ Delivered and green: - The manifest `custom-resource` env resolver is union-aware (fixes the OIDC path); masq log-redaction is wired in `internal/logx`. +After merging `origin/main`, the same envelope was extended to the new +**notification** config (`spec.wandb.notifications`): email `sink`, SMTP +`host`/`port`/`username`/`password`, and Slack `clientId`/`clientSecret` are now +`ValueOrSecret` too — normalized and validated through the same helpers, with the +SMTP resolver and email-sink materialization using `ResolveValue`. (`status.emailSink` +stays a plain `SecretKeySelector` — it is operator-written output.) + Still open: whether to **reject** a literal on strictly-secret fields (deferred, see [Open decisions](#open-decisions)). diff --git a/internal/controller/reconciler/email.go b/internal/controller/reconciler/email.go index 5675ebdc..3171342e 100644 --- a/internal/controller/reconciler/email.go +++ b/internal/controller/reconciler/email.go @@ -44,14 +44,20 @@ func reconcileEmailSink( return deleteGeneratedEmailSink(ctx, client, wandb) } - // Use the sink given by the user - if emailSpec.Sink != nil { - wandb.Status.EmailSink = emailSpec.Sink.DeepCopy() - // The sink is already present under the same name, no need for changes - if emailSpec.Sink.Name == emailSinkSecretName(wandb) { - return nil + // Use the sink given by the user. + if emailSpec.Sink != nil && !emailSpec.Sink.IsZero() { + // A secret-ref sink is passed through to status unchanged. + if ref := emailSpec.Sink.SecretKeyRef(); ref != nil { + wandb.Status.EmailSink = ref.DeepCopy() + // Already the generated secret name means nothing to clean up. + if ref.Name == emailSinkSecretName(wandb) { + return nil + } + return deleteGeneratedEmailSink(ctx, client, wandb) } - return deleteGeneratedEmailSink(ctx, client, wandb) + // A literal sink URL is materialized into the generated Secret so the + // app always consumes a SecretKeyRef. + return writeEmailSinkSecret(ctx, client, wandb, emailSpec.Sink.Value) } // email spec exists, but neither sink nor smtp was supplied @@ -60,15 +66,23 @@ func reconcileEmailSink( return deleteGeneratedEmailSink(ctx, client, wandb) } - // Read the SMTP Secret values and build the sink URL + // Read the SMTP values and build the sink URL. sink, err := resolveSMTPURL(ctx, client, wandb.Namespace, emailSpec.SMTP) if err != nil { return fmt.Errorf("resolve SMTP configuration: %w", err) } + return writeEmailSinkSecret(ctx, client, wandb, sink) +} +// writeEmailSinkSecret materializes sink into the operator-owned email-sink +// Secret and points status.emailSink at it. +func writeEmailSinkSecret( + ctx context.Context, + client ctrlClient.Client, + wandb *apiv2.WeightsAndBiases, + sink string, +) error { secretName := emailSinkSecretName(wandb) - // Creates a new desired secret under the same namespace as the wandb application and under the `sink` key - // belonging to W&B newSecret := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ Name: secretName, @@ -87,7 +101,7 @@ func reconcileEmailSink( } actual := &corev1.Secret{} - err = client.Get(ctx, types.NamespacedName{Name: secretName, Namespace: wandb.Namespace}, actual) + err := client.Get(ctx, types.NamespacedName{Name: secretName, Namespace: wandb.Namespace}, actual) if err != nil && !apierrors.IsNotFound(err) { return fmt.Errorf("get email sink Secret: %w", err) } @@ -116,20 +130,20 @@ func resolveSMTPURL( smtp *apiv2.EmailSMTPSpec, ) (string, error) { - host, err := external.ResolveSecretKey(ctx, client, namespace, smtp.Host) + host, err := external.ResolveValue(ctx, client, namespace, smtp.Host) if err != nil { return "", fmt.Errorf("host: %w", err) } - port, err := external.ResolveSecretKey(ctx, client, namespace, smtp.Port) + port, err := external.ResolveValue(ctx, client, namespace, smtp.Port) if err != nil { return "", fmt.Errorf("port: %w", err) } - username, err := external.ResolveSecretKey(ctx, client, namespace, smtp.Username) + username, err := external.ResolveValue(ctx, client, namespace, smtp.Username) if err != nil { return "", fmt.Errorf("username: %w", err) } - password, err := external.ResolveSecretKey(ctx, client, namespace, smtp.Password) + password, err := external.ResolveValue(ctx, client, namespace, smtp.Password) if err != nil { return "", fmt.Errorf("password: %w", err) } diff --git a/internal/controller/reconciler/email_test.go b/internal/controller/reconciler/email_test.go index 409cc408..8a40760a 100644 --- a/internal/controller/reconciler/email_test.go +++ b/internal/controller/reconciler/email_test.go @@ -21,6 +21,12 @@ func emailTestSelector(name, key string) corev1.SecretKeySelector { } } +// emailTestValue is the ValueOrSecret counterpart for the notification spec +// fields (Sink/SMTP/Slack); emailTestSelector remains for status.emailSink. +func emailTestValue(name, key string) apiv2.ValueOrSecret { + return apiv2.ValueFromSecret(name, key, false) +} + func emailTestWandb(t *testing.T) (*runtime.Scheme, *apiv2.WeightsAndBiases) { t.Helper() scheme := runtime.NewScheme() @@ -44,7 +50,7 @@ func emailTestWandb(t *testing.T) (*runtime.Scheme, *apiv2.WeightsAndBiases) { func TestReconcileEmailSinkUsesConfiguredSink(t *testing.T) { scheme, wandb := emailTestWandb(t) - selector := emailTestSelector("existing-email", "url") + selector := emailTestValue("existing-email", "url") wandb.Spec.Wandb.Notifications.Email = &apiv2.EmailSpec{Sink: &selector} client := fake.NewClientBuilder().WithScheme(scheme).WithObjects(wandb).Build() @@ -58,11 +64,11 @@ func TestReconcileEmailSinkUsesConfiguredSink(t *testing.T) { func TestReconcileEmailSinkDoesNotDeleteConfiguredSink(t *testing.T) { scheme, wandb := emailTestWandb(t) - selector := emailTestSelector("wandb-email-sink", "sink") + selector := emailTestValue("wandb-email-sink", "sink") wandb.Spec.Wandb.Notifications.Email = &apiv2.EmailSpec{Sink: &selector} configured := &corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ - Name: selector.Name, + Name: selector.SecretKeyRef().Name, Namespace: wandb.Namespace, OwnerReferences: []metav1.OwnerReference{{ APIVersion: apiv2.GroupVersion.String(), @@ -108,11 +114,11 @@ func TestResolveEnvvarsCustomResourceEmailSink(t *testing.T) { func TestReconcileEmailSinkGeneratesAuthenticatedSMTPURL(t *testing.T) { scheme, wandb := emailTestWandb(t) - username := emailTestSelector("smtp", "username") - password := emailTestSelector("smtp", "password") + username := emailTestValue("smtp", "username") + password := emailTestValue("smtp", "password") wandb.Spec.Wandb.Notifications.Email = &apiv2.EmailSpec{SMTP: &apiv2.EmailSMTPSpec{ - Host: emailTestSelector("smtp", "host"), - Port: emailTestSelector("smtp", "port"), + Host: emailTestValue("smtp", "host"), + Port: emailTestValue("smtp", "port"), Username: username, Password: password, }} diff --git a/internal/controller/reconciler/slack_env_test.go b/internal/controller/reconciler/slack_env_test.go index 761dc7f6..b1b81528 100644 --- a/internal/controller/reconciler/slack_env_test.go +++ b/internal/controller/reconciler/slack_env_test.go @@ -14,8 +14,8 @@ func TestResolveEnvvarsCustomResourceSlack(t *testing.T) { wandb := &apiv2.WeightsAndBiases{ObjectMeta: metav1.ObjectMeta{Name: "wandb", Namespace: "default"}} wandb.Spec.Wandb.Notifications = &apiv2.NotificationsSpec{} wandb.Spec.Wandb.Notifications.Slack = &apiv2.SlackSpec{ - ClientID: emailTestSelector("slack", "client-id"), - ClientSecret: emailTestSelector("slack", "client-secret"), + ClientID: emailTestValue("slack", "client-id"), + ClientSecret: emailTestValue("slack", "client-secret"), } envs := []serverManifest.EnvVar{ { diff --git a/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml b/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml index f9c9db06..e57b62e7 100644 --- a/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml +++ b/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml @@ -4608,14 +4608,28 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic smtp: properties: host: @@ -4623,53 +4637,109 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic password: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic port: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic username: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic required: - host - password @@ -4684,27 +4754,55 @@ spec: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic clientSecret: properties: key: type: string name: - default: "" type: string optional: type: boolean - required: - - key + value: + type: string + valueFrom: + properties: + secretKeyRef: + properties: + key: + type: string + name: + default: "" + type: string + optional: + type: boolean + required: + - key + type: object + x-kubernetes-map-type: atomic + type: object type: object - x-kubernetes-map-type: atomic type: object type: object oidc: diff --git a/internal/webhook/v2/weightsandbiases_notifications_test.go b/internal/webhook/v2/weightsandbiases_notifications_test.go index 7ae2b62a..c5d4bc0d 100644 --- a/internal/webhook/v2/weightsandbiases_notifications_test.go +++ b/internal/webhook/v2/weightsandbiases_notifications_test.go @@ -5,14 +5,10 @@ import ( "testing" appsv2 "github.com/wandb/operator/api/v2" - corev1 "k8s.io/api/core/v1" ) -func notificationSelector(name, key string) corev1.SecretKeySelector { - return corev1.SecretKeySelector{ - LocalObjectReference: corev1.LocalObjectReference{Name: name}, - Key: key, - } +func notificationSelector(name, key string) appsv2.ValueOrSecret { + return appsv2.ValueFromSecret(name, key, false) } func TestValidateNotificationSpec(t *testing.T) { @@ -43,7 +39,7 @@ func TestValidateNotificationSpec(t *testing.T) { {"Slack missing secret", func(w *appsv2.WeightsAndBiases) { w.Spec.Wandb.Notifications = &appsv2.NotificationsSpec{} w.Spec.Wandb.Notifications.Slack = &appsv2.SlackSpec{ClientID: notificationSelector("slack", "client-id")} - }, "secret name is required"}, + }, "a value or secret reference is required"}, {"email sink", func(w *appsv2.WeightsAndBiases) { w.Spec.Wandb.Notifications = &appsv2.NotificationsSpec{} w.Spec.Wandb.Notifications.Email = &appsv2.EmailSpec{Sink: &sink} @@ -62,10 +58,10 @@ func TestValidateNotificationSpec(t *testing.T) { }, "exactly one"}, {"SMTP missing password", func(w *appsv2.WeightsAndBiases) { smtp := validSMTP() - smtp.Password = corev1.SecretKeySelector{} + smtp.Password = appsv2.ValueOrSecret{} w.Spec.Wandb.Notifications = &appsv2.NotificationsSpec{} w.Spec.Wandb.Notifications.Email = &appsv2.EmailSpec{SMTP: smtp} - }, "secret name is required"}, + }, "a value or secret reference is required"}, } for _, tc := range cases { diff --git a/internal/webhook/v2/weightsandbiases_webhook.go b/internal/webhook/v2/weightsandbiases_webhook.go index 4b17c162..ed72b174 100644 --- a/internal/webhook/v2/weightsandbiases_webhook.go +++ b/internal/webhook/v2/weightsandbiases_webhook.go @@ -387,8 +387,10 @@ func validateNotificationSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { if slack := notifications.Slack; slack != nil { slackPath := base.Child("slack") - errors = append(errors, validateRequiredSecretSelector(slack.ClientID, slackPath.Child("clientId"))...) - errors = append(errors, validateRequiredSecretSelector(slack.ClientSecret, slackPath.Child("clientSecret"))...) + errors = append(errors, validateRequiredValueOrSecret(slack.ClientID, slackPath.Child("clientId"))...) + errors = append(errors, validateValueOrSecret(slack.ClientID, slackPath.Child("clientId"))...) + errors = append(errors, validateRequiredValueOrSecret(slack.ClientSecret, slackPath.Child("clientSecret"))...) + errors = append(errors, validateValueOrSecret(slack.ClientSecret, slackPath.Child("clientSecret"))...) } email := notifications.Email @@ -405,15 +407,23 @@ func validateNotificationSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { return errors } if email.Sink != nil { - errors = append(errors, validateRequiredSecretSelector(*email.Sink, emailPath.Child("sink"))...) + sinkPath := emailPath.Child("sink") + errors = append(errors, validateRequiredValueOrSecret(*email.Sink, sinkPath)...) + errors = append(errors, validateValueOrSecret(*email.Sink, sinkPath)...) return errors } smtpPath := emailPath.Child("smtp") - errors = append(errors, validateRequiredSecretSelector(email.SMTP.Host, smtpPath.Child("host"))...) - errors = append(errors, validateRequiredSecretSelector(email.SMTP.Port, smtpPath.Child("port"))...) - errors = append(errors, validateRequiredSecretSelector(email.SMTP.Username, smtpPath.Child("username"))...) - errors = append(errors, validateRequiredSecretSelector(email.SMTP.Password, smtpPath.Child("password"))...) + for _, f := range []struct { + name string + val appsv2.ValueOrSecret + }{ + {"host", email.SMTP.Host}, {"port", email.SMTP.Port}, + {"username", email.SMTP.Username}, {"password", email.SMTP.Password}, + } { + errors = append(errors, validateRequiredValueOrSecret(f.val, smtpPath.Child(f.name))...) + errors = append(errors, validateValueOrSecret(f.val, smtpPath.Child(f.name))...) + } return errors } @@ -605,20 +615,6 @@ func validateRequiredValueOrSecret(v appsv2.ValueOrSecret, path *field.Path) fie return nil } -// validateRequiredSecretSelector requires a plain SecretKeySelector's name and -// key. Used by fields that are secret-only (e.g. notification config), not the -// value-or-secret connection fields. -func validateRequiredSecretSelector(selector corev1.SecretKeySelector, path *field.Path) field.ErrorList { - var errors field.ErrorList - if selector.Name == "" { - errors = append(errors, field.Required(path.Child("name"), "secret name is required")) - } - if selector.Key == "" { - errors = append(errors, field.Required(path.Child("key"), "secret key is required")) - } - return errors -} - // validateValueOrSecret enforces that a ValueOrSecret sets at most one of a // literal value or a secret reference, and that a ValueFrom carries a // secretKeyRef. It does not require a value; callers enforce requiredness. The @@ -714,6 +710,7 @@ func normalizeConnections(wandb *appsv2.WeightsAndBiases) { spec.ExternalObjectStore.Normalize() } wandb.Spec.Wandb.OIDC.Normalize() + wandb.Spec.Wandb.Notifications.Normalize() } // validateObjectStoreConnection checks value-or-secret exclusivity on each From 6930a24d7ffcb672e072c2891691293bc2f58fb9 Mon Sep 17 00:00:00 2001 From: Daniel Panzella Date: Thu, 20 Aug 2026 12:02:41 -0700 Subject: [PATCH 3/3] chore(deps): Update golang.org/x/mod to 0.40.0 Bumps golang.org/x/mod to v0.40.0; go get pulled its required golang.org/x siblings up as well (crypto v0.55.0, net v0.58.0, text v0.41.0, tools v0.49.0). make lint + make test pass. Co-Authored-By: Claude Opus 4.8 --- go.mod | 10 +++++----- go.sum | 20 ++++++++++---------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 305d7c48..7eeba982 100644 --- a/go.mod +++ b/go.mod @@ -34,7 +34,7 @@ require ( github.com/twmb/franz-go v1.21.3 github.com/twmb/franz-go/pkg/kadm v1.18.0 golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 - golang.org/x/text v0.40.0 + golang.org/x/text v0.41.0 gopkg.in/d4l3k/messagediff.v1 v1.2.1 gopkg.in/yaml.v3 v3.0.1 helm.sh/helm/v3 v3.19.2 @@ -204,15 +204,15 @@ require ( go.uber.org/zap v1.27.1 // indirect go.yaml.in/yaml/v2 v2.4.4 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.54.0 // indirect - golang.org/x/mod v0.38.0 // indirect - golang.org/x/net v0.57.0 // indirect + golang.org/x/crypto v0.55.0 // indirect + golang.org/x/mod v0.40.0 // indirect + golang.org/x/net v0.58.0 // indirect golang.org/x/oauth2 v0.36.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/sys v0.47.0 // indirect golang.org/x/term v0.45.0 // indirect golang.org/x/time v0.15.0 // indirect - golang.org/x/tools v0.48.0 // indirect + golang.org/x/tools v0.49.0 // indirect gomodules.xyz/jsonpatch/v2 v2.5.0 // indirect google.golang.org/genproto/googleapis/api v0.0.0-20260414002931-afd174a4e478 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20260414002931-afd174a4e478 // indirect diff --git a/go.sum b/go.sum index d04b3ea7..aea7632f 100644 --- a/go.sum +++ b/go.sum @@ -524,22 +524,22 @@ golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5y golang.org/x/crypto v0.0.0-20220722155217-630584e8d5aa/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4= golang.org/x/crypto v0.6.0/go.mod h1:OFC/31mSvZgRz0V1QTNCzfAI1aIRzbiufJtkMIlEp58= golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= -golang.org/x/crypto v0.54.0 h1:YLIA59K4fiNzHzjnZt2tUJQjQtUWfWbeHBqKtk3eScw= -golang.org/x/crypto v0.54.0/go.mod h1:KWL8ny2AZdGR2cWmzeHrp2azQPGogOv+HeQaVEXC2dk= +golang.org/x/crypto v0.55.0 h1:+KWHjbgOaAQ66dh/YlkZKHlz9ZUlq61AFirAR9ntP8M= +golang.org/x/crypto v0.55.0/go.mod h1:uq0V9dE/fzQuJtbnL+2EhWOE63vo164FY8xqEnV9xis= golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0 h1:R84qjqJb5nVJMxqWYb3np9L5ZsaDtB+a39EqjV0JSUM= golang.org/x/exp v0.0.0-20250408133849-7e4ce0ab07d0/go.mod h1:S9Xr4PYopiDyqSyp5NjCrhFrqg6A5zA2E/iPHPhqnS8= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= -golang.org/x/mod v0.38.0 h1:MECBjubtXD7yj4HrhIUcywNaGeNVUdfVnxmPajOk4yk= -golang.org/x/mod v0.38.0/go.mod h1:V6Xz0pq8TQ3dGqVQ1FVHuelZpAL0uNhSkk9ogYP3c40= +golang.org/x/mod v0.40.0 h1:hUv+3cXcdRHz08UmSiOob7sadHig73uo5bkXxQ/tvUs= +golang.org/x/mod v0.40.0/go.mod h1:0/weTWkPWGBikyTWAX3dkjVztMmBA5hM0DH6BElSupE= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs= golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg= -golang.org/x/net v0.57.0 h1:K5+3DljvIuDG9/Jv9rvyMywYNFCQ9RSUY6OOTTkT+tE= -golang.org/x/net v0.57.0/go.mod h1:KpXc8iv+r3XplLAG/f7Jsf9RPszJzdR0f58q9vGOuEU= +golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To= +golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= golang.org/x/oauth2 v0.36.0 h1:peZ/1z27fi9hUOFCAZaHyrpWG5lwe0RJEEEeH0ThlIs= golang.org/x/oauth2 v0.36.0/go.mod h1:YDBUJMTkDnJS+A4BP4eZBjCqtokkg1hODuPjwiGPO7Q= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -574,16 +574,16 @@ golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ= golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs= -golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= -golang.org/x/tools v0.48.0 h1:3+hClM1aLL5mjMKm5ovokw9epgRXPuu2tILgismM6RE= -golang.org/x/tools v0.48.0/go.mod h1:08xX0orndb/F7jJxGDicx061tyd5pcMto75YMAXr6lk= +golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= +golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= gomodules.xyz/jsonpatch/v2 v2.5.0 h1:JELs8RLM12qJGXU4u/TO3V25KW8GreMKl9pdkk14RM0= gomodules.xyz/jsonpatch/v2 v2.5.0/go.mod h1:AH3dM2RI6uoBZxn3LVrfvJ3E0/9dG4cSrbuBJT4moAY=