diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 7edc029a..cb7d2893 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -17,7 +17,9 @@ limitations under the License. package main import ( + "context" "crypto/tls" + "encoding/json" "flag" "fmt" "log/slog" @@ -286,6 +288,14 @@ func main() { os.Exit(1) } + // Non-fatal: a cluster that blocks the discovery endpoint should still start. + // The reconciler surfaces a not-ready condition rather than asserting an + // issuer it never verified. + if err := RegisterServiceAccountIssuer(context.Background()); err != nil { + setupLog.Error(err, "failed to discover the cluster service-account issuer; "+ + "internal service auth will not reconcile until spec.wandb.internalServiceAuth.oidcIssuer is set") + } + mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ Scheme: scheme, Cache: cacheOptions, @@ -445,6 +455,47 @@ func detectRuntimeNamespace() (string, error) { return namespace, nil } +// serviceAccountIssuerDiscoveryPath is the API server's OIDC discovery document. +// Its `issuer` field is the value the API server stamps as `iss` on projected +// ServiceAccount tokens. +const serviceAccountIssuerDiscoveryPath = "/.well-known/openid-configuration" + +// RegisterServiceAccountIssuer discovers the cluster's service-account issuer so +// W&B services can be told the exact issuer their projected tokens carry. The +// issuer is a fixed property of the API server's --service-account-issuer flag, +// so one lookup at start-up is enough; granting the operator access after the +// fact requires a restart. +func RegisterServiceAccountIssuer(ctx context.Context) error { + cfg, err := config.GetConfig() + if err != nil { + return err + } + + discoveryClient, err := discovery.NewDiscoveryClientForConfig(cfg) + if err != nil { + return err + } + + raw, err := discoveryClient.RESTClient().Get().AbsPath(serviceAccountIssuerDiscoveryPath).DoRaw(ctx) + if err != nil { + return fmt.Errorf("get %s: %w", serviceAccountIssuerDiscoveryPath, err) + } + + var doc struct { + Issuer string `json:"issuer"` + } + if err := json.Unmarshal(raw, &doc); err != nil { + return fmt.Errorf("decode %s: %w", serviceAccountIssuerDiscoveryPath, err) + } + if doc.Issuer == "" { + return fmt.Errorf("%s returned no issuer field", serviceAccountIssuerDiscoveryPath) + } + + setupLog.Info("discovered cluster service-account issuer", "issuer", doc.Issuer) + utils.SetServiceAccountIssuer(doc.Issuer) + return nil +} + func RegisterServerResources() error { cfg, err := config.GetConfig() if err != nil { diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 22dac7bc..f55f5bac 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -5,6 +5,7 @@ metadata: name: manager-role rules: - nonResourceURLs: + - /.well-known/openid-configuration - /metrics verbs: - get diff --git a/deploy/operator/templates/wandb-operator-wandb-role.yaml b/deploy/operator/templates/wandb-operator-wandb-role.yaml index 290ae4e8..90514ea9 100644 --- a/deploy/operator/templates/wandb-operator-wandb-role.yaml +++ b/deploy/operator/templates/wandb-operator-wandb-role.yaml @@ -145,6 +145,13 @@ rules: - patch - update - watch + # Read the cluster's service-account issuer. Internal service auth must be told + # the API server's exact --service-account-issuer, or W&B services reject each + # other's projected tokens with a 401. + - nonResourceURLs: + - /.well-known/openid-configuration + verbs: + - get --- apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRoleBinding diff --git a/internal/controller/reconciler/pods.go b/internal/controller/reconciler/pods.go index ec03d584..a1afa667 100644 --- a/internal/controller/reconciler/pods.go +++ b/internal/controller/reconciler/pods.go @@ -410,12 +410,10 @@ func resolveEnvvars(ctx context.Context, client ctrlClient.Client, wandb *v2.Wei } components = append(components, fmt.Sprintf("%s%s:%d%s", proto, svcHost, selectedPort, src.Path)) case "jwt-issuer-map": - if wandb.Spec.Wandb.InternalServiceAuth.Enabled != nil && - *wandb.Spec.Wandb.InternalServiceAuth.Enabled { - // TODO Get real OIDC Issuer - issuer := "https://kubernetes.default.svc.cluster.local" - if wandb.Spec.Wandb.InternalServiceAuth.OIDCIssuer != "" { - issuer = wandb.Spec.Wandb.InternalServiceAuth.OIDCIssuer + if internalServiceAuthEnabled(wandb) { + issuer := resolveInternalServiceAuthIssuer(wandb) + if issuer == "" { + continue } components = append( components, diff --git a/internal/controller/reconciler/reconcile_v2.go b/internal/controller/reconciler/reconcile_v2.go index 409a35d2..fef2bf98 100644 --- a/internal/controller/reconciler/reconcile_v2.go +++ b/internal/controller/reconciler/reconcile_v2.go @@ -521,6 +521,18 @@ func ReconcileWandbManifest( return ctrl.Result{RequeueAfter: 5 * time.Second}, nil } + // Refuse to reconcile applications with an unknown issuer rather than emitting + // a guessed one: W&B services reject a mismatched `iss` with a 401 and the API + // panics on that path, so no value is safer than a wrong value. + if internalServiceAuthEnabled(wandb) && resolveInternalServiceAuthIssuer(wandb) == "" { + logger.Info("Cluster service-account issuer unknown; not reconciling applications") + if err := updateReadyStatus(ctx, client, wandb, statusBefore, false, + serviceAccountIssuerUnknownReason, serviceAccountIssuerUnknownMessage); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{RequeueAfter: time.Minute}, nil + } + result, err = reconcileApplications(ctx, client, wandb, manifest, telemetryConfig) if err != nil { return result, err diff --git a/internal/controller/weightsandbiases_controller.go b/internal/controller/weightsandbiases_controller.go index 965264ee..30b3fe12 100644 --- a/internal/controller/weightsandbiases_controller.go +++ b/internal/controller/weightsandbiases_controller.go @@ -90,6 +90,9 @@ type WeightsAndBiasesReconciler struct { //+kubebuilder:rbac:groups=redis.redis.opstreelabs.in,resources=redis/status,verbs=get //+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,resourceNames=nonroot-v2,verbs=use //+kubebuilder:rbac:urls=/metrics,verbs=get +// Read the cluster's service-account issuer, which internal service auth must +// match exactly (see RegisterServiceAccountIssuer). +//+kubebuilder:rbac:urls=/.well-known/openid-configuration,verbs=get // Deprecated/Erroneously required RBAC rules //+kubebuilder:rbac:groups=extensions,resources=daemonsets;deployments;replicasets;ingresses;ingresses/status,verbs=get;list;watch diff --git a/internal/webhook/v2/weightsandbiases_webhook.go b/internal/webhook/v2/weightsandbiases_webhook.go index fd229ca4..62ca6f4b 100644 --- a/internal/webhook/v2/weightsandbiases_webhook.go +++ b/internal/webhook/v2/weightsandbiases_webhook.go @@ -110,9 +110,11 @@ func (d *WeightsAndBiasesCustomDefaulter) Default(ctx context.Context, obj runti wandb.Spec.Wandb.InternalServiceAuth.Enabled = ptr.To(true) } - if wandb.Spec.Wandb.InternalServiceAuth.OIDCIssuer == "" { - wandb.Spec.Wandb.InternalServiceAuth.OIDCIssuer = "https://kubernetes.default.svc.cluster.local" - } + // OIDCIssuer is deliberately not defaulted: it must match the API server's + // --service-account-issuer, which only kubeadm sets to + // kubernetes.default.svc.cluster.local. Defaulting it here would mask the + // issuer discovered from the cluster and pin every install to a value that + // fails token validation on EKS/GKE/AKS. if wandb.Spec.Wandb.ServiceAccount.Create == nil { wandb.Spec.Wandb.ServiceAccount.Create = ptr.To(true) diff --git a/internal/webhook/v2/weightsandbiases_webhook_test.go b/internal/webhook/v2/weightsandbiases_webhook_test.go index 8bb0d2ae..7996bc2d 100644 --- a/internal/webhook/v2/weightsandbiases_webhook_test.go +++ b/internal/webhook/v2/weightsandbiases_webhook_test.go @@ -64,7 +64,8 @@ var _ = Describe("WeightsAndBiases Webhook", func() { Expect(obj.Spec.Wandb.ManifestRepository).To(Equal("oci://example.com/wandb/server-manifest")) Expect(obj.Spec.Wandb.InternalServiceAuth.Enabled).ToNot(BeNil()) Expect(*obj.Spec.Wandb.InternalServiceAuth.Enabled).To(BeTrue()) - Expect(obj.Spec.Wandb.InternalServiceAuth.OIDCIssuer).To(Equal("https://kubernetes.default.svc.cluster.local")) + Expect(obj.Spec.Wandb.InternalServiceAuth.OIDCIssuer).To(BeEmpty(), + "oidcIssuer must not be defaulted; it has to match the cluster's --service-account-issuer") Expect(obj.Spec.Wandb.ServiceAccount.Create).ToNot(BeNil()) Expect(*obj.Spec.Wandb.ServiceAccount.Create).To(BeTrue()) Expect(obj.Spec.Wandb.ServiceAccount.ServiceAccountName).To(Equal("wandb-app"))