diff --git a/api/v2/weightsandbiases_types.go b/api/v2/weightsandbiases_types.go index 1655793f..0d2fa1d0 100644 --- a/api/v2/weightsandbiases_types.go +++ b/api/v2/weightsandbiases_types.go @@ -17,6 +17,8 @@ limitations under the License. package v2 import ( + "strings" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -138,6 +140,94 @@ type WeightsAndBiasesSpec struct { // Networking configures how the W&B application is exposed externally. // +optional Networking NetworkingSpec `json:"networking,omitempty"` + + // Watchtower configures the in-cluster Watchtower admin UI. + // +optional + Watchtower WatchtowerSpec `json:"watchtower,omitempty"` +} + +// WatchtowerSpec configures the operator-managed Watchtower deployment. +// +// Watchtower is served under spec.watchtower.basePath on the same hostname as +// the W&B app so the browser's existing session cookie can be validated against +// the app's /oidc/auth endpoint — the same piggyback the deprecated console used. +// It is not published in the server manifest and versions independently of W&B, +// so its image is configured here rather than resolved from the manifest. +type WatchtowerSpec struct { + Install *bool `json:"install,omitempty"` + Image WatchtowerImageSpec `json:"image,omitempty"` + BasePath string `json:"basePath,omitempty"` + AuthService string `json:"authService,omitempty"` + Resources corev1.ResourceRequirements `json:"resources,omitempty"` + ServiceAccount ManagedServiceAccountSpec `json:"serviceAccount,omitempty"` +} + +// WatchtowerImageSpec identifies the Watchtower container image. Digest wins +// over Tag when both are set. +type WatchtowerImageSpec struct { + // +optional + Repository string `json:"repository,omitempty"` + // +optional + Tag string `json:"tag,omitempty"` + // +optional + Digest string `json:"digest,omitempty"` +} + +const ( + // DefaultWatchtowerImageRepository is the published Watchtower image. + DefaultWatchtowerImageRepository = "us-docker.pkg.dev/wandb-production/public/wandb/watchtower" + // DefaultWatchtowerImageTag pins the Watchtower version this operator ships + // against. Watchtower releases independently of W&B, so bumping it is an + // operator change. + DefaultWatchtowerImageTag = "0.11.0" + // DefaultWatchtowerBasePath is the URL prefix Watchtower is served under, + // mirroring how console was mounted at /console. + DefaultWatchtowerBasePath = "/watchtower" + // DefaultWatchtowerServiceAccountName is the ServiceAccount Watchtower runs as. + DefaultWatchtowerServiceAccountName = "wandb-watchtower" +) + +// WatchtowerEnabled reports whether Watchtower should be deployed. +func (w *WeightsAndBiases) WatchtowerEnabled() bool { + return w.Spec.Watchtower.Install != nil && *w.Spec.Watchtower.Install +} + +// GetImage returns the fully qualified Watchtower image, retargeted to the +// global image registry when one is configured for air-gapped installs. +func (s WatchtowerSpec) GetImage(globalImageRegistry string) string { + repository := s.Repository() + if globalImageRegistry != "" { + repository = globalImageRegistry + "/" + repository + } + if s.Image.Digest != "" { + return repository + "@" + s.Image.Digest + } + tag := s.Image.Tag + if tag == "" { + tag = DefaultWatchtowerImageTag + } + return repository + ":" + tag +} + +// Repository returns the configured image repository or the published default. +func (s WatchtowerSpec) Repository() string { + if s.Image.Repository != "" { + return s.Image.Repository + } + return DefaultWatchtowerImageRepository +} + +// ResolvedBasePath returns the URL prefix Watchtower is served under, without a +// trailing slash so callers can concatenate paths onto it. +func (s WatchtowerSpec) ResolvedBasePath() string { + basePath := s.BasePath + if basePath == "" { + basePath = DefaultWatchtowerBasePath + } + if !strings.HasPrefix(basePath, "/") { + basePath = "/" + basePath + } + return strings.TrimSuffix(basePath, "/") } // GlobalSpec holds settings shared across every managed component. @@ -756,6 +846,19 @@ type WeightsAndBiasesStatus struct { GatewayStatus *GatewayStatusSummary `json:"gatewayStatus,omitempty"` // +optional IngressStatus *IngressStatusSummary `json:"ingressStatus,omitempty"` + + // WatchtowerStatus reports the Watchtower deployment. Nil when Watchtower is + // not installed. Watchtower is an optional admin UI, so it is kept out of + // status.wandb.applications and never gates the Ready condition. + // +optional + WatchtowerStatus *WatchtowerStatusSummary `json:"watchtowerStatus,omitempty"` +} + +type WatchtowerStatusSummary struct { + Ready bool `json:"ready"` + URL string `json:"url,omitempty"` + Image string `json:"image,omitempty"` + AuthService string `json:"authService,omitempty"` } type GatewayStatusSummary struct { diff --git a/api/v2/zz_generated.deepcopy.go b/api/v2/zz_generated.deepcopy.go index 76ab543b..8195f562 100644 --- a/api/v2/zz_generated.deepcopy.go +++ b/api/v2/zz_generated.deepcopy.go @@ -1620,6 +1620,59 @@ func (in *WandbStatus) DeepCopy() *WandbStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WatchtowerImageSpec) DeepCopyInto(out *WatchtowerImageSpec) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WatchtowerImageSpec. +func (in *WatchtowerImageSpec) DeepCopy() *WatchtowerImageSpec { + if in == nil { + return nil + } + out := new(WatchtowerImageSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WatchtowerSpec) DeepCopyInto(out *WatchtowerSpec) { + *out = *in + if in.Install != nil { + in, out := &in.Install, &out.Install + *out = new(bool) + **out = **in + } + out.Image = in.Image + in.Resources.DeepCopyInto(&out.Resources) + in.ServiceAccount.DeepCopyInto(&out.ServiceAccount) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WatchtowerSpec. +func (in *WatchtowerSpec) DeepCopy() *WatchtowerSpec { + if in == nil { + return nil + } + out := new(WatchtowerSpec) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *WatchtowerStatusSummary) DeepCopyInto(out *WatchtowerStatusSummary) { + *out = *in +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WatchtowerStatusSummary. +func (in *WatchtowerStatusSummary) DeepCopy() *WatchtowerStatusSummary { + if in == nil { + return nil + } + out := new(WatchtowerStatusSummary) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *WeightsAndBiases) DeepCopyInto(out *WeightsAndBiases) { *out = *in @@ -1731,6 +1784,7 @@ func (in *WeightsAndBiasesSpec) DeepCopyInto(out *WeightsAndBiasesSpec) { } } in.Networking.DeepCopyInto(&out.Networking) + in.Watchtower.DeepCopyInto(&out.Watchtower) } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WeightsAndBiasesSpec. @@ -1801,6 +1855,11 @@ func (in *WeightsAndBiasesStatus) DeepCopyInto(out *WeightsAndBiasesStatus) { *out = new(IngressStatusSummary) (*in).DeepCopyInto(*out) } + if in.WatchtowerStatus != nil { + in, out := &in.WatchtowerStatus, &out.WatchtowerStatus + *out = new(WatchtowerStatusSummary) + **out = **in + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new WeightsAndBiasesStatus. diff --git a/config/crd/bases/apps.wandb.com_weightsandbiases.yaml b/config/crd/bases/apps.wandb.com_weightsandbiases.yaml index 4ee11815..4a8faa17 100644 --- a/config/crd/bases/apps.wandb.com_weightsandbiases.yaml +++ b/config/crd/bases/apps.wandb.com_weightsandbiases.yaml @@ -4405,6 +4405,68 @@ spec: - hostname - version type: object + watchtower: + properties: + authService: + type: string + basePath: + type: string + image: + properties: + digest: + type: string + repository: + type: string + tag: + type: string + type: object + install: + type: boolean + resources: + properties: + claims: + items: + properties: + name: + type: string + request: + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object + type: object + serviceAccount: + properties: + annotations: + additionalProperties: + type: string + type: object + create: + type: boolean + serviceAccountName: + type: string + type: object + type: object required: - retentionPolicy type: object @@ -6356,6 +6418,19 @@ spec: required: - hostname type: object + watchtowerStatus: + properties: + authService: + type: string + image: + type: string + ready: + type: boolean + url: + type: string + required: + - ready + type: object required: - observedGeneration - ready diff --git a/internal/controller/reconciler/ingress.go b/internal/controller/reconciler/ingress.go index 75410b57..da6905a3 100644 --- a/internal/controller/reconciler/ingress.go +++ b/internal/controller/reconciler/ingress.go @@ -92,6 +92,10 @@ func reconcileConsolidatedIngress(ctx context.Context, c ctrlClient.Client, wand }) } + if watchtowerPath := watchtowerIngressPath(wandb); watchtowerPath != nil { + paths = append(paths, *watchtowerPath) + } + if len(paths) == 0 { return nil } diff --git a/internal/controller/reconciler/reconcile_v2.go b/internal/controller/reconciler/reconcile_v2.go index 2da77ebf..7e227a34 100644 --- a/internal/controller/reconciler/reconcile_v2.go +++ b/internal/controller/reconciler/reconcile_v2.go @@ -42,6 +42,7 @@ import ( "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" "k8s.io/client-go/tools/record" + "k8s.io/utils/ptr" ctrl "sigs.k8s.io/controller-runtime" ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -259,6 +260,12 @@ func Reconcile( if err = deleteInfraHTTPRoutes(ctx, client, wandb); err != nil { return ctrl.Result{}, err } + // Watchtower's ClusterRole and ClusterRoleBinding are cluster-scoped + // and cannot own-reference the CR, so garbage collection would leave + // them behind. + if err = deleteWatchtower(ctx, client, wandb); err != nil { + return ctrl.Result{}, err + } if wandb.Spec.Networking.Mode == apiv2.NetworkingModeIngress { if err = deleteConsolidatedIngress(ctx, client, wandb); err != nil { return ctrl.Result{}, err @@ -418,6 +425,14 @@ func ReconcileWandbManifest( statusBefore := wandb.DeepCopy().Status + // Reconciled ahead of the infra gate below: Watchtower is the UI customers use + // to diagnose a deploy, so it has to come up even when the install it manages + // is stuck. It does not depend on any backing service. + if err := reconcileWatchtower(ctx, client, wandb, manifest); err != nil { + logger.Error(err, "Failed to reconcile Watchtower") + return ctrl.Result{}, err + } + redisReady := redisAllReady(wandb) mysqlReady := mysqlAllReady(wandb) kafkaReady := wandb.Status.KafkaStatus.Ready @@ -807,17 +822,41 @@ func applicationManagedFieldsEqual(before, after *apiv2.Application) bool { } func buildHTTPRouteTemplate(wandb *apiv2.WeightsAndBiases, app serverManifest.Application) *apiv2.HTTPRouteTemplateSpec { + var paths []string + var pathType string + if app.Ingress != nil { + paths = app.Ingress.Paths + pathType = app.Ingress.PathType + } + + return buildHTTPRouteTemplateForPaths(wandb, paths, pathType, resolveHTTPRouteServicePort(app)) +} + +// buildHTTPRouteTemplateForPaths builds an HTTPRoute template against the CR's +// gateway and hostnames for an arbitrary set of paths, so operator-owned +// components (not just manifest applications) can be routed. +func buildHTTPRouteTemplateForPaths( + wandb *apiv2.WeightsAndBiases, + paths []string, + pathType string, + servicePort *gatewayv1.PortNumber, +) *apiv2.HTTPRouteTemplateSpec { gwConfig := wandb.Spec.Networking.GatewayAPI ref := wandb.Status.GatewayStatus.GatewayRef parentRef := gatewayv1.ParentReference{ Name: gatewayv1.ObjectName(ref.Name), + // Spelled out even though these are the schema defaults: otherwise the + // stored Application never equals the one we build, and the update gate + // fires on every reconcile. + Group: ptr.To(gatewayv1.Group(gatewayv1.GroupName)), + Kind: ptr.To(gatewayv1.Kind("Gateway")), } if ref.Namespace != "" && ref.Namespace != wandb.Namespace { ns := gatewayv1.Namespace(ref.Namespace) parentRef.Namespace = &ns } - if gwConfig.ListenerName != nil { + if gwConfig != nil && gwConfig.ListenerName != nil { sectionName := gatewayv1.SectionName(*gwConfig.ListenerName) parentRef.SectionName = §ionName } @@ -828,19 +867,12 @@ func buildHTTPRouteTemplate(wandb *apiv2.WeightsAndBiases, app serverManifest.Ap hostnames = append(hostnames, gatewayv1.Hostname(h)) } - var paths []string - var pathType string - if app.Ingress != nil { - paths = app.Ingress.Paths - pathType = app.Ingress.PathType - } - return &apiv2.HTTPRouteTemplateSpec{ ParentRefs: []gatewayv1.ParentReference{parentRef}, Hostnames: hostnames, Paths: paths, PathType: pathType, - ServicePort: resolveHTTPRouteServicePort(app), + ServicePort: servicePort, } } diff --git a/internal/controller/reconciler/watchtower.go b/internal/controller/reconciler/watchtower.go new file mode 100644 index 00000000..11925bcf --- /dev/null +++ b/internal/controller/reconciler/watchtower.go @@ -0,0 +1,508 @@ +package reconciler + +import ( + "context" + "fmt" + "slices" + "strings" + + apiv2 "github.com/wandb/operator/api/v2" + "github.com/wandb/operator/internal/controller/common" + "github.com/wandb/operator/internal/logx" + "github.com/wandb/operator/pkg/utils" + serverManifest "github.com/wandb/operator/pkg/wandb/manifest" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + rbacv1 "k8s.io/api/rbac/v1" + apiErrors "k8s.io/apimachinery/pkg/api/errors" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" + controllerruntime "sigs.k8s.io/controller-runtime" + ctrlClient "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +const ( + // watchtowerComponent labels every Watchtower resource so the manifest-driven + // Application pruning skips it and stale resources stay findable. + watchtowerComponent = "watchtower" + + // watchtowerAppName names the Application, and therefore the Deployment and + // Service the application controller derives from it. Prefixed so it cannot + // collide with an application the server manifest may publish later. + watchtowerAppName = "wandb-watchtower" + + watchtowerContainerPort = int32(8080) + watchtowerPortName = "http" + + // watchtowerOIDCIngressPath is the ingress path owned by the application that + // serves gorilla's /oidc/auth sub-request, used to derive AUTH_SERVICE. + watchtowerOIDCIngressPath = "/oidc" +) + +// reconcileWatchtower brings the operator-managed Watchtower deployment in line +// with spec.watchtower. Watchtower is not published in the server manifest, so +// the operator owns its Application, Service account and RBAC outright. +func reconcileWatchtower( + ctx context.Context, + c ctrlClient.Client, + wandb *apiv2.WeightsAndBiases, + manifest serverManifest.Manifest, +) error { + logger := logx.GetSlog(ctx) + + if !wandb.WatchtowerEnabled() { + return deleteWatchtower(ctx, c, wandb) + } + + authService, err := watchtowerAuthService(wandb, manifest) + if err != nil { + return err + } + + if err := reconcileWatchtowerServiceAccount(ctx, c, wandb); err != nil { + return err + } + if err := reconcileWatchtowerRBAC(ctx, c, wandb); err != nil { + return err + } + + desired := buildWatchtowerApplication(wandb, authService) + + application := &apiv2.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: watchtowerAppName, + Namespace: wandb.Namespace, + }, + } + op, err := controllerruntime.CreateOrUpdate(ctx, c, application, func() error { + application.Labels = utils.MergeMapsStringString(application.Labels, desired.Labels) + application.Spec = desired.Spec + return controllerutil.SetOwnerReference(wandb, application, c.Scheme()) + }) + if err != nil { + return fmt.Errorf("failed to reconcile Watchtower Application: %w", err) + } + logger.Info(fmt.Sprintf("Successfully %s Watchtower Application", op), + "application", watchtowerAppName, "authService", authService) + + wandb.Status.WatchtowerStatus = &apiv2.WatchtowerStatusSummary{ + Ready: application.Status.Ready, + URL: watchtowerURL(wandb), + Image: wandb.Spec.Watchtower.GetImage(wandb.Spec.Global.ImageRegistry), + AuthService: authService, + } + + return nil +} + +// watchtowerURL is where a browser reaches Watchtower: the W&B hostname plus the +// base path, since it is deliberately served from the app's own origin. +func watchtowerURL(wandb *apiv2.WeightsAndBiases) string { + hostname := strings.TrimSuffix(wandb.Spec.Wandb.Hostname, "/") + if hostname == "" { + return "" + } + if !strings.Contains(hostname, "://") { + hostname = "https://" + hostname + } + return hostname + wandb.Spec.Watchtower.ResolvedBasePath() +} + +// deleteWatchtower removes every Watchtower resource. The cluster-scoped +// ClusterRole and ClusterRoleBinding cannot carry an owner reference to a +// namespaced CR, so they are deleted here explicitly rather than by GC. +func deleteWatchtower(ctx context.Context, c ctrlClient.Client, wandb *apiv2.WeightsAndBiases) error { + objects := []ctrlClient.Object{ + &apiv2.Application{ObjectMeta: metav1.ObjectMeta{Name: watchtowerAppName, Namespace: wandb.Namespace}}, + &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: watchtowerAppName, Namespace: wandb.Namespace}}, + &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: watchtowerAppName, Namespace: wandb.Namespace}}, + &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: watchtowerClusterScopedName(wandb)}}, + &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: watchtowerClusterScopedName(wandb)}}, + } + + for _, obj := range objects { + if err := c.Delete(ctx, obj); err != nil && !apiErrors.IsNotFound(err) { + return fmt.Errorf("failed to delete Watchtower %T %s: %w", obj, obj.GetName(), err) + } + } + + // The ServiceAccount is owner-referenced and only deleted when the operator + // created it; a user-supplied account is left alone. + if ptr.Deref(wandb.Spec.Watchtower.ServiceAccount.Create, true) { + sa := &corev1.ServiceAccount{ObjectMeta: metav1.ObjectMeta{ + Name: watchtowerServiceAccountName(wandb), + Namespace: wandb.Namespace, + }} + if err := c.Delete(ctx, sa); err != nil && !apiErrors.IsNotFound(err) { + return fmt.Errorf("failed to delete Watchtower ServiceAccount: %w", err) + } + } + + wandb.Status.WatchtowerStatus = nil + return nil +} + +func buildWatchtowerApplication(wandb *apiv2.WeightsAndBiases, authService string) *apiv2.Application { + watchtower := wandb.Spec.Watchtower + labels := watchtowerLabels(wandb) + basePath := watchtower.ResolvedBasePath() + + app := &apiv2.Application{ + ObjectMeta: metav1.ObjectMeta{ + Name: watchtowerAppName, + Namespace: wandb.Namespace, + Labels: labels, + }, + Spec: apiv2.ApplicationSpec{ + Kind: "Deployment", + // Pinned to one replica: in-flight deploy jobs and their SSE streams + // live in the serving pod's memory, so a reconnect that lands on a + // second pod would see no history. + Replicas: ptr.To(int32(1)), + MetaTemplate: metav1.ObjectMeta{ + Labels: labels, + }, + PodTemplate: corev1.PodTemplateSpec{ + Spec: corev1.PodSpec{ + ServiceAccountName: watchtowerServiceAccountName(wandb), + SecurityContext: resolvePodSecurityContext(), + Affinity: wandb.Spec.Affinity, + Tolerations: watchtowerTolerations(wandb), + Containers: []corev1.Container{ + { + Name: watchtowerComponent, + Image: watchtower.GetImage(wandb.Spec.Global.ImageRegistry), + SecurityContext: resolveContainerSecurityContext(), + Env: watchtowerEnv(wandb, authService, basePath), + Resources: watchtowerResources(watchtower), + Ports: []corev1.ContainerPort{{ + Name: watchtowerPortName, + ContainerPort: watchtowerContainerPort, + Protocol: corev1.ProtocolTCP, + }}, + // Probes go through the base path because the server + // mounts every route, health included, behind it. + LivenessProbe: watchtowerProbe(basePath + "/healthz"), + ReadinessProbe: watchtowerProbe(basePath + "/ready"), + }, + }, + }, + }, + ServiceTemplate: &corev1.ServiceSpec{ + Type: corev1.ServiceTypeClusterIP, + Ports: []corev1.ServicePort{{ + Name: watchtowerPortName, + Port: watchtowerContainerPort, + TargetPort: intstr.FromInt32(watchtowerContainerPort), + Protocol: corev1.ProtocolTCP, + }}, + }, + }, + } + + if wandb.Spec.Networking.Mode == apiv2.NetworkingModeGatewayAPI && + wandb.Status.GatewayStatus != nil && wandb.Status.GatewayStatus.GatewayRef != nil { + app.Spec.HTTPRouteTemplate = buildHTTPRouteTemplateForPaths( + wandb, + []string{basePath}, + string(networkingv1.PathTypePrefix), + ptr.To(gatewayv1.PortNumber(watchtowerContainerPort)), + ) + } + + return app +} + +// watchtowerEnv is the operator's side of the contract with the Watchtower +// container: it is told where it is mounted and which service validates the +// caller's session, so neither has to be baked into the image. +func watchtowerEnv(wandb *apiv2.WeightsAndBiases, authService, basePath string) []corev1.EnvVar { + return []corev1.EnvVar{ + // Locks the UI to the cluster it runs in: no context switching, no teardown. + {Name: "WATCHTOWER_MODE", Value: "cluster"}, + {Name: "WATCHTOWER_BASE_PATH", Value: basePath}, + {Name: "WATCHTOWER_AUTH_SERVICE", Value: authService}, + {Name: "WATCHTOWER_WANDB_NAME", Value: wandb.Name}, + {Name: "WATCHTOWER_NAMESPACE", ValueFrom: &corev1.EnvVarSource{ + FieldRef: &corev1.ObjectFieldSelector{FieldPath: "metadata.namespace"}, + }}, + } +} + +// watchtowerAuthService resolves the in-cluster host:port Watchtower calls to +// validate the browser's W&B session cookie. The manifest application that owns +// the /oidc ingress path is the one serving gorilla's /oidc/auth, and the +// application controller names its Service after the application, so this stays +// correct across manifest renames. +// +// Failing here is deliberate: without an auth service Watchtower would serve +// cluster administration unauthenticated. spec.watchtower.authService is the +// escape hatch for deployments whose manifest does not declare the path. +func watchtowerAuthService(wandb *apiv2.WeightsAndBiases, manifest serverManifest.Manifest) (string, error) { + if wandb.Spec.Watchtower.AuthService != "" { + return wandb.Spec.Watchtower.AuthService, nil + } + + for _, app := range sortedManifestApplications(manifest) { + if app.Ingress == nil || app.Service == nil { + continue + } + if len(app.Features) > 0 && !manifest.FeaturesEnabled(app.Features) { + continue + } + if !slices.Contains(app.Ingress.Paths, watchtowerOIDCIngressPath) { + continue + } + return fmt.Sprintf("%s:%d", app.Name, watchtowerAuthServicePort(app)), nil + } + + return "", fmt.Errorf( + "cannot derive spec.watchtower.authService: no manifest application serves the %q ingress path; set it explicitly", + watchtowerOIDCIngressPath, + ) +} + +// watchtowerAuthServicePort resolves the app's ingress service port to a number, +// following the named-port indirection the manifest allows. +func watchtowerAuthServicePort(app serverManifest.Application) int32 { + if app.Ingress != nil && app.Ingress.ServicePort != "" { + parsed := intstr.Parse(app.Ingress.ServicePort) + if parsed.Type == intstr.Int { + return parsed.IntVal + } + for _, port := range app.Service.Ports { + if port.Name == parsed.StrVal { + return port.Port + } + } + } + if len(app.Service.Ports) > 0 { + return app.Service.Ports[0].Port + } + return watchtowerContainerPort +} + +// watchtowerIngressPath returns the consolidated-Ingress path for Watchtower, or +// nil when it is not installed. +func watchtowerIngressPath(wandb *apiv2.WeightsAndBiases) *networkingv1.HTTPIngressPath { + if !wandb.WatchtowerEnabled() { + return nil + } + pathType := networkingv1.PathTypePrefix + return &networkingv1.HTTPIngressPath{ + Path: wandb.Spec.Watchtower.ResolvedBasePath(), + PathType: &pathType, + Backend: networkingv1.IngressBackend{ + Service: &networkingv1.IngressServiceBackend{ + Name: watchtowerAppName, + Port: networkingv1.ServiceBackendPort{Number: watchtowerContainerPort}, + }, + }, + } +} + +func watchtowerProbe(path string) *corev1.Probe { + return &corev1.Probe{ + ProbeHandler: corev1.ProbeHandler{ + HTTPGet: &corev1.HTTPGetAction{ + Path: path, + Port: intstr.FromInt32(watchtowerContainerPort), + }, + }, + TimeoutSeconds: 3, + PeriodSeconds: 10, + FailureThreshold: 3, + } +} + +// watchtowerResources keeps the UI modest by default; it is an admin console +// whose heavy work happens in the cluster, not in this pod. +func watchtowerResources(watchtower apiv2.WatchtowerSpec) corev1.ResourceRequirements { + if len(watchtower.Resources.Requests) > 0 || len(watchtower.Resources.Limits) > 0 { + return watchtower.Resources + } + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: resource.MustParse("100m"), + corev1.ResourceMemory: resource.MustParse("256Mi"), + }, + } +} + +func watchtowerTolerations(wandb *apiv2.WeightsAndBiases) []corev1.Toleration { + if wandb.Spec.Tolerations == nil { + return nil + } + return *wandb.Spec.Tolerations +} + +func watchtowerServiceAccountName(wandb *apiv2.WeightsAndBiases) string { + if name := wandb.Spec.Watchtower.ServiceAccount.ServiceAccountName; name != "" { + return name + } + return apiv2.DefaultWatchtowerServiceAccountName +} + +// watchtowerClusterScopedName qualifies cluster-scoped RBAC with the CR's +// namespace so two W&B installs in one cluster do not fight over one object. +func watchtowerClusterScopedName(wandb *apiv2.WeightsAndBiases) string { + return fmt.Sprintf("%s-%s-watchtower", wandb.Namespace, wandb.Name) +} + +func watchtowerLabels(wandb *apiv2.WeightsAndBiases) map[string]string { + labels := common.BuildWandbLabels(wandb, watchtowerComponent) + labels["app.kubernetes.io/managed-by"] = "wandb-operator" + labels["app.kubernetes.io/instance"] = wandb.Name + labels["app.kubernetes.io/part-of"] = "wandb" + return labels +} + +func reconcileWatchtowerServiceAccount(ctx context.Context, c ctrlClient.Client, wandb *apiv2.WeightsAndBiases) error { + if !ptr.Deref(wandb.Spec.Watchtower.ServiceAccount.Create, true) { + return nil + } + + serviceAccount := &corev1.ServiceAccount{ + ObjectMeta: metav1.ObjectMeta{ + Name: watchtowerServiceAccountName(wandb), + Namespace: wandb.Namespace, + }, + } + _, err := controllerruntime.CreateOrUpdate(ctx, c, serviceAccount, func() error { + serviceAccount.Labels = utils.MergeMapsStringString(serviceAccount.Labels, watchtowerLabels(wandb)) + serviceAccount.Annotations = utils.MergeMapsStringString( + serviceAccount.Annotations, + wandb.Spec.Watchtower.ServiceAccount.Annotations, + ) + // Watchtower talks to the Kubernetes API with this token, so unlike the + // W&B application pods it must have one mounted. + serviceAccount.AutomountServiceAccountToken = ptr.To(true) + return controllerutil.SetControllerReference(wandb, serviceAccount, c.Scheme()) + }) + if err != nil { + return fmt.Errorf("failed to reconcile Watchtower ServiceAccount: %w", err) + } + return nil +} + +// reconcileWatchtowerRBAC grants Watchtower what the operator itself holds and +// can therefore delegate: the apiserver rejects a binding that would escalate +// beyond the operator's own permissions. +// +// Deliberately absent, because the operator does not hold them today: +// apiextensions.k8s.io/customresourcedefinitions (v2 served-version detection) +// and pods/portforward (telemetry port-forward). Both need the operator's own +// ClusterRole widened first. +func reconcileWatchtowerRBAC(ctx context.Context, c ctrlClient.Client, wandb *apiv2.WeightsAndBiases) error { + labels := watchtowerLabels(wandb) + serviceAccountName := watchtowerServiceAccountName(wandb) + clusterScopedName := watchtowerClusterScopedName(wandb) + + role := &rbacv1.Role{ObjectMeta: metav1.ObjectMeta{Name: watchtowerAppName, Namespace: wandb.Namespace}} + if _, err := controllerruntime.CreateOrUpdate(ctx, c, role, func() error { + role.Labels = utils.MergeMapsStringString(role.Labels, labels) + // Secrets and ConfigMaps stay namespace-scoped: Watchtower reads the + // install's license and connection material, not the whole cluster's. + role.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets", "configmaps"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + APIGroups: []string{"batch"}, + Resources: []string{"jobs", "cronjobs"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + APIGroups: []string{"networking.k8s.io"}, + Resources: []string{"ingresses"}, + Verbs: []string{"get", "list", "watch"}, + }, + } + return controllerutil.SetOwnerReference(wandb, role, c.Scheme()) + }); err != nil { + return fmt.Errorf("failed to reconcile Watchtower Role: %w", err) + } + + roleBinding := &rbacv1.RoleBinding{ObjectMeta: metav1.ObjectMeta{Name: watchtowerAppName, Namespace: wandb.Namespace}} + if _, err := controllerruntime.CreateOrUpdate(ctx, c, roleBinding, func() error { + roleBinding.Labels = utils.MergeMapsStringString(roleBinding.Labels, labels) + roleBinding.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "Role", + Name: watchtowerAppName, + } + roleBinding.Subjects = []rbacv1.Subject{{ + Kind: "ServiceAccount", + Name: serviceAccountName, + Namespace: wandb.Namespace, + }} + return controllerutil.SetOwnerReference(wandb, roleBinding, c.Scheme()) + }); err != nil { + return fmt.Errorf("failed to reconcile Watchtower RoleBinding: %w", err) + } + + clusterRole := &rbacv1.ClusterRole{ObjectMeta: metav1.ObjectMeta{Name: clusterScopedName}} + if _, err := controllerruntime.CreateOrUpdate(ctx, c, clusterRole, func() error { + clusterRole.Labels = utils.MergeMapsStringString(clusterRole.Labels, labels) + clusterRole.Rules = []rbacv1.PolicyRule{ + { + APIGroups: []string{"apps.wandb.com"}, + Resources: []string{"weightsandbiases"}, + Verbs: []string{"get", "list", "watch", "update", "patch"}, + }, + { + APIGroups: []string{"apps.wandb.com"}, + Resources: []string{"applications"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + // Only "get": the operator itself holds get/update/patch on these + // subresources, and it cannot grant verbs it does not have. + APIGroups: []string{"apps.wandb.com"}, + Resources: []string{"weightsandbiases/status", "applications/status"}, + Verbs: []string{"get"}, + }, + { + APIGroups: []string{""}, + Resources: []string{"namespaces", "pods", "pods/log", "services", "events"}, + Verbs: []string{"get", "list", "watch"}, + }, + { + APIGroups: []string{"apps"}, + Resources: []string{"deployments", "statefulsets", "replicasets", "daemonsets"}, + Verbs: []string{"get", "list", "watch"}, + }, + } + // Cluster-scoped objects cannot own-reference a namespaced CR; cleanup + // runs through deleteWatchtower instead. + return nil + }); err != nil { + return fmt.Errorf("failed to reconcile Watchtower ClusterRole: %w", err) + } + + clusterRoleBinding := &rbacv1.ClusterRoleBinding{ObjectMeta: metav1.ObjectMeta{Name: clusterScopedName}} + if _, err := controllerruntime.CreateOrUpdate(ctx, c, clusterRoleBinding, func() error { + clusterRoleBinding.Labels = utils.MergeMapsStringString(clusterRoleBinding.Labels, labels) + clusterRoleBinding.RoleRef = rbacv1.RoleRef{ + APIGroup: rbacv1.GroupName, + Kind: "ClusterRole", + Name: clusterScopedName, + } + clusterRoleBinding.Subjects = []rbacv1.Subject{{ + Kind: "ServiceAccount", + Name: serviceAccountName, + Namespace: wandb.Namespace, + }} + return nil + }); err != nil { + return fmt.Errorf("failed to reconcile Watchtower ClusterRoleBinding: %w", err) + } + + return nil +} diff --git a/internal/controller/reconciler/watchtower_test.go b/internal/controller/reconciler/watchtower_test.go new file mode 100644 index 00000000..56bbc691 --- /dev/null +++ b/internal/controller/reconciler/watchtower_test.go @@ -0,0 +1,393 @@ +package reconciler + +import ( + "context" + "slices" + "testing" + + "github.com/stretchr/testify/require" + apiv2 "github.com/wandb/operator/api/v2" + "github.com/wandb/operator/internal/controller/common" + serverManifest "github.com/wandb/operator/pkg/wandb/manifest" + corev1 "k8s.io/api/core/v1" + networkingv1 "k8s.io/api/networking/v1" + rbacv1 "k8s.io/api/rbac/v1" + apiErrors "k8s.io/apimachinery/pkg/api/errors" + 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" + gatewayv1 "sigs.k8s.io/gateway-api/apis/v1" +) + +func watchtowerTestClient(t *testing.T, objects ...ctrlClient.Object) ctrlClient.Client { + t.Helper() + scheme := runtime.NewScheme() + require.NoError(t, apiv2.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, rbacv1.AddToScheme(scheme)) + + builder := fake.NewClientBuilder().WithScheme(scheme) + if len(objects) > 0 { + builder.WithObjects(objects...) + } + return builder.Build() +} + +func newWandbForWatchtower(install bool) *apiv2.WeightsAndBiases { + return &apiv2.WeightsAndBiases{ + TypeMeta: metav1.TypeMeta{APIVersion: "apps.wandb.com/v2", Kind: "WeightsAndBiases"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "wandb", + Namespace: "default", + UID: "wandb-uid", + }, + Spec: apiv2.WeightsAndBiasesSpec{ + Wandb: apiv2.WandbAppSpec{Hostname: "https://wandb.example.com"}, + Watchtower: apiv2.WatchtowerSpec{ + Install: ptr.To(install), + }, + }, + } +} + +// watchtowerTestManifest mirrors the published manifest's shape: the api +// application owns /oidc and therefore the auth sub-request endpoint. +func watchtowerTestManifest() serverManifest.Manifest { + return serverManifest.Manifest{ + Applications: map[string]serverManifest.Application{ + "api": { + Name: "api", + Service: &serverManifest.ServiceSpec{ + Ports: []corev1.ServicePort{{Name: "api", Port: 8080}}, + }, + Ingress: &serverManifest.AppIngressSpec{ + Paths: []string{"/api", "/graphql", "/oidc"}, + ServicePort: "8080", + PathType: "Prefix", + }, + }, + "frontend": { + Name: "frontend", + Service: &serverManifest.ServiceSpec{ + Ports: []corev1.ServicePort{{Name: "frontend", Port: 8080}}, + }, + Ingress: &serverManifest.AppIngressSpec{ + Paths: []string{"/"}, + ServicePort: "8080", + PathType: "Prefix", + }, + }, + }, + } +} + +func TestWatchtowerAuthServiceDerivedFromManifest(t *testing.T) { + wandb := newWandbForWatchtower(true) + + authService, err := watchtowerAuthService(wandb, watchtowerTestManifest()) + require.NoError(t, err) + require.Equal(t, "api:8080", authService) +} + +func TestWatchtowerAuthServiceResolvesNamedServicePort(t *testing.T) { + manifest := watchtowerTestManifest() + api := manifest.Applications["api"] + api.Ingress.ServicePort = "api" + api.Service.Ports = []corev1.ServicePort{{Name: "api", Port: 8081}} + manifest.Applications["api"] = api + + authService, err := watchtowerAuthService(newWandbForWatchtower(true), manifest) + require.NoError(t, err) + require.Equal(t, "api:8081", authService) +} + +func TestWatchtowerAuthServiceSpecOverrideWins(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Watchtower.AuthService = "wandb-api:8081" + + authService, err := watchtowerAuthService(wandb, watchtowerTestManifest()) + require.NoError(t, err) + require.Equal(t, "wandb-api:8081", authService) +} + +// Failing closed matters here: a Watchtower with no auth service would serve +// cluster administration to unauthenticated callers. +func TestWatchtowerAuthServiceErrorsWhenUnderivable(t *testing.T) { + manifest := watchtowerTestManifest() + delete(manifest.Applications, "api") + + _, err := watchtowerAuthService(newWandbForWatchtower(true), manifest) + require.ErrorContains(t, err, "cannot derive spec.watchtower.authService") +} + +func TestWatchtowerAuthServiceSkipsDisabledFeatureApps(t *testing.T) { + manifest := watchtowerTestManifest() + api := manifest.Applications["api"] + api.Features = []string{"unavailable"} + manifest.Applications["api"] = api + + _, err := watchtowerAuthService(newWandbForWatchtower(true), manifest) + require.ErrorContains(t, err, "cannot derive spec.watchtower.authService") +} + +func TestReconcileWatchtowerCreatesApplicationAndRBAC(t *testing.T) { + wandb := newWandbForWatchtower(true) + c := watchtowerTestClient(t) + + require.NoError(t, reconcileWatchtower(context.Background(), c, wandb, watchtowerTestManifest())) + + app := &apiv2.Application{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, app)) + + require.Equal(t, "Deployment", app.Spec.Kind) + require.Equal(t, int32(1), *app.Spec.Replicas) + require.Equal(t, watchtowerComponent, app.Labels[common.WandbComponentLabel]) + require.Len(t, app.Spec.PodTemplate.Spec.Containers, 1) + + container := app.Spec.PodTemplate.Spec.Containers[0] + require.Equal(t, apiv2.DefaultWatchtowerImageRepository+":"+apiv2.DefaultWatchtowerImageTag, container.Image) + require.Equal(t, "/watchtower/healthz", container.LivenessProbe.HTTPGet.Path) + require.Equal(t, "/watchtower/ready", container.ReadinessProbe.HTTPGet.Path) + + env := map[string]string{} + for _, e := range container.Env { + env[e.Name] = e.Value + } + require.Equal(t, "cluster", env["WATCHTOWER_MODE"]) + require.Equal(t, "/watchtower", env["WATCHTOWER_BASE_PATH"]) + require.Equal(t, "api:8080", env["WATCHTOWER_AUTH_SERVICE"]) + + require.Equal(t, apiv2.DefaultWatchtowerServiceAccountName, app.Spec.PodTemplate.Spec.ServiceAccountName) + require.NotNil(t, app.Spec.ServiceTemplate) + require.Equal(t, watchtowerContainerPort, app.Spec.ServiceTemplate.Ports[0].Port) + + sa := &corev1.ServiceAccount{} + require.NoError(t, c.Get(context.Background(), types.NamespacedName{ + Name: apiv2.DefaultWatchtowerServiceAccountName, Namespace: "default"}, sa)) + require.True(t, *sa.AutomountServiceAccountToken) + + role := &rbacv1.Role{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, role)) + + clusterRole := &rbacv1.ClusterRole{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: "default-wandb-watchtower"}, clusterRole)) + + clusterRoleBinding := &rbacv1.ClusterRoleBinding{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: "default-wandb-watchtower"}, clusterRoleBinding)) + require.Equal(t, apiv2.DefaultWatchtowerServiceAccountName, clusterRoleBinding.Subjects[0].Name) + + // The apiserver rejects a binding that grants more than the operator holds, + // and the operator only has "get" on these subresources. + for _, rule := range clusterRole.Rules { + if slices.Contains(rule.Resources, "weightsandbiases/status") { + require.Equal(t, []string{"get"}, rule.Verbs) + } + } + + require.NotNil(t, wandb.Status.WatchtowerStatus) + require.Equal(t, "https://wandb.example.com/watchtower", wandb.Status.WatchtowerStatus.URL) + require.Equal(t, "api:8080", wandb.Status.WatchtowerStatus.AuthService) +} + +func TestReconcileWatchtowerIsIdempotent(t *testing.T) { + wandb := newWandbForWatchtower(true) + c := watchtowerTestClient(t) + ctx := context.Background() + + require.NoError(t, reconcileWatchtower(ctx, c, wandb, watchtowerTestManifest())) + first := &apiv2.Application{} + require.NoError(t, c.Get(ctx, types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, first)) + + require.NoError(t, reconcileWatchtower(ctx, c, wandb, watchtowerTestManifest())) + second := &apiv2.Application{} + require.NoError(t, c.Get(ctx, types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, second)) + + require.Equal(t, first.ResourceVersion, second.ResourceVersion) +} + +func TestReconcileWatchtowerHonorsGlobalImageRegistry(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Global.ImageRegistry = "registry.internal" + c := watchtowerTestClient(t) + + require.NoError(t, reconcileWatchtower(context.Background(), c, wandb, watchtowerTestManifest())) + + app := &apiv2.Application{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, app)) + require.Equal(t, + "registry.internal/"+apiv2.DefaultWatchtowerImageRepository+":"+apiv2.DefaultWatchtowerImageTag, + app.Spec.PodTemplate.Spec.Containers[0].Image, + ) +} + +func TestReconcileWatchtowerBuildsHTTPRouteInGatewayMode(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Networking.Mode = apiv2.NetworkingModeGatewayAPI + wandb.Spec.Networking.GatewayAPI = &apiv2.GatewayAPIConfig{} + wandb.Status.GatewayStatus = &apiv2.GatewayStatusSummary{ + GatewayRef: &apiv2.GatewayReference{Name: "wandb-gateway"}, + } + c := watchtowerTestClient(t) + + require.NoError(t, reconcileWatchtower(context.Background(), c, wandb, watchtowerTestManifest())) + + app := &apiv2.Application{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, app)) + + route := app.Spec.HTTPRouteTemplate + require.NotNil(t, route) + require.Equal(t, []string{"/watchtower"}, route.Paths) + require.Equal(t, []gatewayv1.Hostname{"wandb.example.com"}, route.Hostnames) + require.Equal(t, gatewayv1.PortNumber(watchtowerContainerPort), *route.ServicePort) +} + +func TestReconcileWatchtowerOmitsHTTPRouteInIngressMode(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Networking.Mode = apiv2.NetworkingModeIngress + c := watchtowerTestClient(t) + + require.NoError(t, reconcileWatchtower(context.Background(), c, wandb, watchtowerTestManifest())) + + app := &apiv2.Application{} + require.NoError(t, c.Get(context.Background(), + types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, app)) + require.Nil(t, app.Spec.HTTPRouteTemplate) +} + +func TestReconcileWatchtowerDisabledRemovesResources(t *testing.T) { + wandb := newWandbForWatchtower(true) + c := watchtowerTestClient(t) + ctx := context.Background() + + require.NoError(t, reconcileWatchtower(ctx, c, wandb, watchtowerTestManifest())) + + wandb.Spec.Watchtower.Install = ptr.To(false) + require.NoError(t, reconcileWatchtower(ctx, c, wandb, watchtowerTestManifest())) + + err := c.Get(ctx, types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, &apiv2.Application{}) + require.True(t, apiErrors.IsNotFound(err)) + + err = c.Get(ctx, types.NamespacedName{Name: "default-wandb-watchtower"}, &rbacv1.ClusterRole{}) + require.True(t, apiErrors.IsNotFound(err), "cluster-scoped RBAC has no owner ref, so it must be deleted explicitly") + + err = c.Get(ctx, types.NamespacedName{Name: "default-wandb-watchtower"}, &rbacv1.ClusterRoleBinding{}) + require.True(t, apiErrors.IsNotFound(err)) + + err = c.Get(ctx, types.NamespacedName{ + Name: apiv2.DefaultWatchtowerServiceAccountName, Namespace: "default"}, &corev1.ServiceAccount{}) + require.True(t, apiErrors.IsNotFound(err)) + + require.Nil(t, wandb.Status.WatchtowerStatus) +} + +func TestReconcileWatchtowerDisabledIsANoopWhenAbsent(t *testing.T) { + wandb := newWandbForWatchtower(false) + c := watchtowerTestClient(t) + + require.NoError(t, reconcileWatchtower(context.Background(), c, wandb, watchtowerTestManifest())) + require.Nil(t, wandb.Status.WatchtowerStatus) +} + +func TestReconcileWatchtowerKeepsUserSuppliedServiceAccount(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Watchtower.ServiceAccount = apiv2.ManagedServiceAccountSpec{ + Create: ptr.To(false), + ServiceAccountName: "byo-watchtower", + } + c := watchtowerTestClient(t) + ctx := context.Background() + + require.NoError(t, reconcileWatchtower(ctx, c, wandb, watchtowerTestManifest())) + + err := c.Get(ctx, types.NamespacedName{Name: "byo-watchtower", Namespace: "default"}, &corev1.ServiceAccount{}) + require.True(t, apiErrors.IsNotFound(err), "the operator must not create an account it was told not to manage") + + app := &apiv2.Application{} + require.NoError(t, c.Get(ctx, types.NamespacedName{Name: watchtowerAppName, Namespace: "default"}, app)) + require.Equal(t, "byo-watchtower", app.Spec.PodTemplate.Spec.ServiceAccountName) +} + +func TestWatchtowerIngressPathDisabled(t *testing.T) { + require.Nil(t, watchtowerIngressPath(newWandbForWatchtower(false))) +} + +func TestWatchtowerIngressPathHonorsBasePath(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Watchtower.BasePath = "/admin/" + + path := watchtowerIngressPath(wandb) + require.NotNil(t, path) + require.Equal(t, "/admin", path.Path) + require.Equal(t, watchtowerAppName, path.Backend.Service.Name) + require.Equal(t, watchtowerContainerPort, path.Backend.Service.Port.Number) +} + +func TestConsolidatedIngressIncludesWatchtowerPath(t *testing.T) { + wandb := newWandbForWatchtower(true) + wandb.Spec.Networking.Mode = apiv2.NetworkingModeIngress + ctx := context.Background() + + scheme := runtime.NewScheme() + require.NoError(t, apiv2.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, networkingv1.AddToScheme(scheme)) + c := fake.NewClientBuilder().WithScheme(scheme).Build() + + require.NoError(t, reconcileConsolidatedIngress(ctx, c, wandb, watchtowerTestManifest())) + + ingress := &networkingv1.Ingress{} + require.NoError(t, c.Get(ctx, types.NamespacedName{Name: "wandb", Namespace: "default"}, ingress)) + + backends := map[string]string{} + for _, path := range ingress.Spec.Rules[0].HTTP.Paths { + backends[path.Path] = path.Backend.Service.Name + } + require.Equal(t, watchtowerAppName, backends["/watchtower"]) + require.Equal(t, "frontend", backends["/"], "the W&B frontend must keep serving the root path") +} + +func TestConsolidatedIngressOmitsWatchtowerPathWhenDisabled(t *testing.T) { + wandb := newWandbForWatchtower(false) + wandb.Spec.Networking.Mode = apiv2.NetworkingModeIngress + ctx := context.Background() + + scheme := runtime.NewScheme() + require.NoError(t, apiv2.AddToScheme(scheme)) + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, networkingv1.AddToScheme(scheme)) + c := fake.NewClientBuilder().WithScheme(scheme).Build() + + require.NoError(t, reconcileConsolidatedIngress(ctx, c, wandb, watchtowerTestManifest())) + + ingress := &networkingv1.Ingress{} + require.NoError(t, c.Get(ctx, types.NamespacedName{Name: "wandb", Namespace: "default"}, ingress)) + for _, path := range ingress.Spec.Rules[0].HTTP.Paths { + require.NotEqual(t, "/watchtower", path.Path) + } +} + +func TestWatchtowerResolvedBasePathNormalizes(t *testing.T) { + require.Equal(t, "/watchtower", apiv2.WatchtowerSpec{}.ResolvedBasePath()) + require.Equal(t, "/admin", apiv2.WatchtowerSpec{BasePath: "admin"}.ResolvedBasePath()) + require.Equal(t, "/admin", apiv2.WatchtowerSpec{BasePath: "/admin/"}.ResolvedBasePath()) +} + +func TestWatchtowerGetImagePrefersDigest(t *testing.T) { + spec := apiv2.WatchtowerSpec{ + Image: apiv2.WatchtowerImageSpec{ + Repository: "wandb/watchtower", + Tag: "0.11.0", + Digest: "sha256:abc", + }, + } + require.Equal(t, "wandb/watchtower@sha256:abc", spec.GetImage("")) +} diff --git a/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml b/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml index 4ee11815..4a8faa17 100644 --- a/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml +++ b/internal/crdinstaller/crds/operator/apps.wandb.com_weightsandbiases.yaml @@ -4405,6 +4405,68 @@ spec: - hostname - version type: object + watchtower: + properties: + authService: + type: string + basePath: + type: string + image: + properties: + digest: + type: string + repository: + type: string + tag: + type: string + type: object + install: + type: boolean + resources: + properties: + claims: + items: + properties: + name: + type: string + request: + type: string + required: + - name + type: object + type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map + limits: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object + requests: + additionalProperties: + anyOf: + - type: integer + - type: string + pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$ + x-kubernetes-int-or-string: true + type: object + type: object + serviceAccount: + properties: + annotations: + additionalProperties: + type: string + type: object + create: + type: boolean + serviceAccountName: + type: string + type: object + type: object required: - retentionPolicy type: object @@ -6356,6 +6418,19 @@ spec: required: - hostname type: object + watchtowerStatus: + properties: + authService: + type: string + image: + type: string + ready: + type: boolean + url: + type: string + required: + - ready + type: object required: - observedGeneration - ready diff --git a/internal/webhook/v2/weightsandbiases_defaulter_watchtower_test.go b/internal/webhook/v2/weightsandbiases_defaulter_watchtower_test.go new file mode 100644 index 00000000..7c567e52 --- /dev/null +++ b/internal/webhook/v2/weightsandbiases_defaulter_watchtower_test.go @@ -0,0 +1,137 @@ +package v2 + +import ( + "context" + + . "github.com/onsi/ginkgo/v2" + g "github.com/onsi/gomega" + apiv2 "github.com/wandb/operator/api/v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" +) + +var _ = Describe("WeightsAndBiasesCustomDefaulter - Watchtower", func() { + var ( + ctx context.Context + defaulter WeightsAndBiasesCustomDefaulter + validator WeightsAndBiasesCustomValidator + ) + + newWandb := func(watchtower apiv2.WatchtowerSpec) *apiv2.WeightsAndBiases { + return &apiv2.WeightsAndBiases{ + ObjectMeta: metav1.ObjectMeta{Name: "test-wandb", Namespace: "test-namespace"}, + Spec: apiv2.WeightsAndBiasesSpec{ + Wandb: apiv2.WandbAppSpec{Hostname: "https://wandb.example.com"}, + Watchtower: watchtower, + }, + } + } + + BeforeEach(func() { + ctx = context.Background() + defaulter = WeightsAndBiasesCustomDefaulter{} + validator = WeightsAndBiasesCustomValidator{} + }) + + It("leaves Watchtower uninstalled by default", func() { + wandb := newWandb(apiv2.WatchtowerSpec{}) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + g.Expect(*wandb.Spec.Watchtower.Install).To(g.BeFalse()) + g.Expect(wandb.WatchtowerEnabled()).To(g.BeFalse()) + // Nothing else is filled in for a component that will not be deployed. + g.Expect(wandb.Spec.Watchtower.BasePath).To(g.BeEmpty()) + g.Expect(wandb.Spec.Watchtower.Image.Repository).To(g.BeEmpty()) + }) + + It("fills image, base path and service account when installed", func() { + wandb := newWandb(apiv2.WatchtowerSpec{Install: ptr.To(true)}) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + g.Expect(wandb.Spec.Watchtower.Image.Repository).To(g.Equal(apiv2.DefaultWatchtowerImageRepository)) + g.Expect(wandb.Spec.Watchtower.Image.Tag).To(g.Equal(apiv2.DefaultWatchtowerImageTag)) + g.Expect(wandb.Spec.Watchtower.BasePath).To(g.Equal(apiv2.DefaultWatchtowerBasePath)) + g.Expect(*wandb.Spec.Watchtower.ServiceAccount.Create).To(g.BeTrue()) + g.Expect(wandb.Spec.Watchtower.ServiceAccount.ServiceAccountName). + To(g.Equal(apiv2.DefaultWatchtowerServiceAccountName)) + }) + + It("preserves an explicit image, base path and service account", func() { + wandb := newWandb(apiv2.WatchtowerSpec{ + Install: ptr.To(true), + BasePath: "/admin", + Image: apiv2.WatchtowerImageSpec{ + Repository: "registry.internal/watchtower", + Tag: "1.2.3", + }, + ServiceAccount: apiv2.ManagedServiceAccountSpec{ + Create: ptr.To(false), + ServiceAccountName: "byo-watchtower", + }, + }) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + g.Expect(wandb.Spec.Watchtower.Image.Repository).To(g.Equal("registry.internal/watchtower")) + g.Expect(wandb.Spec.Watchtower.Image.Tag).To(g.Equal("1.2.3")) + g.Expect(wandb.Spec.Watchtower.BasePath).To(g.Equal("/admin")) + g.Expect(*wandb.Spec.Watchtower.ServiceAccount.Create).To(g.BeFalse()) + g.Expect(wandb.Spec.Watchtower.ServiceAccount.ServiceAccountName).To(g.Equal("byo-watchtower")) + }) + + It("does not default a tag over a pinned digest", func() { + wandb := newWandb(apiv2.WatchtowerSpec{ + Install: ptr.To(true), + Image: apiv2.WatchtowerImageSpec{Digest: "sha256:abc"}, + }) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + g.Expect(wandb.Spec.Watchtower.Image.Tag).To(g.BeEmpty()) + }) + + It("rejects a base path that would shadow the W&B frontend", func() { + wandb := newWandb(apiv2.WatchtowerSpec{Install: ptr.To(true), BasePath: "/"}) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + _, err := validator.ValidateCreate(ctx, wandb) + g.Expect(err).To(g.MatchError(g.ContainSubstring("must not be '/'"))) + }) + + It("rejects a relative base path", func() { + wandb := newWandb(apiv2.WatchtowerSpec{Install: ptr.To(true), BasePath: "watchtower"}) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + _, err := validator.ValidateCreate(ctx, wandb) + g.Expect(err).To(g.MatchError(g.ContainSubstring("must start with '/'"))) + }) + + It("rejects an authService carrying a scheme", func() { + wandb := newWandb(apiv2.WatchtowerSpec{ + Install: ptr.To(true), + AuthService: "http://wandb-api:8081", + }) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + _, err := validator.ValidateCreate(ctx, wandb) + g.Expect(err).To(g.MatchError(g.ContainSubstring("bare host:port"))) + }) + + It("accepts a valid installed Watchtower", func() { + wandb := newWandb(apiv2.WatchtowerSpec{ + Install: ptr.To(true), + AuthService: "wandb-api:8081", + }) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + _, err := validator.ValidateCreate(ctx, wandb) + g.Expect(err).ToNot(g.HaveOccurred()) + }) + + It("skips validation entirely when Watchtower is off", func() { + // An invalid leftover block must not block a CR that does not install it. + wandb := newWandb(apiv2.WatchtowerSpec{Install: ptr.To(false), BasePath: "nonsense"}) + + g.Expect(defaulter.Default(ctx, wandb)).To(g.Succeed()) + _, err := validator.ValidateCreate(ctx, wandb) + g.Expect(err).ToNot(g.HaveOccurred()) + }) +}) diff --git a/internal/webhook/v2/weightsandbiases_webhook.go b/internal/webhook/v2/weightsandbiases_webhook.go index e58bb84b..787146c0 100644 --- a/internal/webhook/v2/weightsandbiases_webhook.go +++ b/internal/webhook/v2/weightsandbiases_webhook.go @@ -132,6 +132,7 @@ func (d *WeightsAndBiasesCustomDefaulter) Default(ctx context.Context, obj runti applyObjectStoreDefaults(wandb) applyClickHouseDefaults(wandb) applyProbeDefaults(wandb) + applyWatchtowerDefaults(wandb) if defaultStore, ok := wandb.Spec.ObjectStore["default"]; ok && defaultStore.ManagedObjectStore != nil { wandb.Spec.Wandb.BucketProxy = true @@ -335,6 +336,32 @@ func applyClickHouseDefaults(wandb *appsv2.WeightsAndBiases) { } } +// applyWatchtowerDefaults fills the Watchtower block. install defaults to false: +// Watchtower grants cluster-wide read access to whoever holds a W&B session, so +// enabling it is an explicit decision. +func applyWatchtowerDefaults(wandb *appsv2.WeightsAndBiases) { + watchtower := &wandb.Spec.Watchtower + + if watchtower.Install == nil { + watchtower.Install = ptr.To(false) + } + + if !*watchtower.Install { + return + } + + if watchtower.Image.Repository == "" { + watchtower.Image.Repository = appsv2.DefaultWatchtowerImageRepository + } + if watchtower.Image.Tag == "" && watchtower.Image.Digest == "" { + watchtower.Image.Tag = appsv2.DefaultWatchtowerImageTag + } + if watchtower.BasePath == "" { + watchtower.BasePath = appsv2.DefaultWatchtowerBasePath + } + applyManagedServiceAccountDefaults(&watchtower.ServiceAccount, appsv2.DefaultWatchtowerServiceAccountName) +} + func applyManagedServiceAccountDefaults(serviceAccount *appsv2.ManagedServiceAccountSpec, defaultName string) { if serviceAccount.Create == nil { serviceAccount.Create = ptr.To(true) @@ -360,6 +387,7 @@ func validateSpec(_ context.Context, newWandb, oldWandb *appsv2.WeightsAndBiases allErrors = append(allErrors, networkingErrors...) warnings = append(warnings, networkingWarnings...) allErrors = append(allErrors, validateProxySpec(newWandb)...) + allErrors = append(allErrors, validateWatchtowerSpec(newWandb)...) if len(allErrors) == 0 { return warnings, nil @@ -477,6 +505,42 @@ func validateWandbSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { return errors } +func validateWatchtowerSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { + var errors field.ErrorList + + if !wandb.WatchtowerEnabled() { + return errors + } + + watchtower := wandb.Spec.Watchtower + watchtowerPath := field.NewPath("spec").Child("watchtower") + + basePath := watchtower.BasePath + switch { + case !strings.HasPrefix(basePath, "/"): + errors = append(errors, field.Invalid( + watchtowerPath.Child("basePath"), basePath, "must start with '/'", + )) + case strings.Trim(basePath, "/") == "": + // "/" is the W&B frontend's own path; mounting Watchtower there would + // shadow the app it is meant to manage. + errors = append(errors, field.Invalid( + watchtowerPath.Child("basePath"), basePath, "must not be '/', which is served by the W&B frontend", + )) + } + + if authService := watchtower.AuthService; authService != "" { + if strings.Contains(authService, "://") || strings.Contains(authService, "/") { + errors = append(errors, field.Invalid( + watchtowerPath.Child("authService"), authService, + "must be a bare host:port, without a scheme or path", + )) + } + } + + return errors +} + func validateMySQLSpec(wandb *appsv2.WeightsAndBiases) field.ErrorList { var errors field.ErrorList mysqlPath := field.NewPath("spec").Child("mysql")