diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e715773e..4e556b28 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -45,6 +45,9 @@ jobs: - name: Helm template tests run: make helm-test + - name: Check release assets + run: make verify-release-manifest + - name: Vet run: make vet diff --git a/.github/workflows/release.yaml b/.github/workflows/release.yaml index 0a641c2e..5277f407 100644 --- a/.github/workflows/release.yaml +++ b/.github/workflows/release.yaml @@ -83,6 +83,17 @@ jobs: echo "version=${VERSION}" >> "${GITHUB_OUTPUT}" echo "tag=v${VERSION}" >> "${GITHUB_OUTPUT}" + - name: Set up Go + uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 + with: + go-version-file: go.mod + cache: true # zizmor: ignore[cache-poisoning] + + - name: Build and check release manifests + env: + TAG: ${{ steps.version.outputs.tag }} + run: make verify-release-manifest IMG="${IMAGE}:${TAG}" + - name: Create and push tag env: TAG: ${{ steps.version.outputs.tag }} @@ -114,12 +125,6 @@ jobs: exit 1 fi - - name: Set up Go - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 - with: - go-version-file: go.mod - cache: true # zizmor: ignore[cache-poisoning] - - name: Build and push container image env: TAG: ${{ steps.version.outputs.tag }} @@ -130,11 +135,6 @@ jobs: podman login -u "${ACTOR}" -p "${GH_TOKEN}" ghcr.io podman push "${IMAGE}:${TAG}" - - name: Build install manifest - env: - TAG: ${{ steps.version.outputs.tag }} - run: make release-manifest IMG="${IMAGE}:${TAG}" - - name: Package and push Helm charts env: VERSION: ${{ steps.version.outputs.version }} @@ -169,6 +169,11 @@ jobs: printf 'kubectl apply -f https://github.com/%s/releases/download/%s/install.yaml\n' \ "${GITHUB_REPOSITORY}" "${TAG}" printf '```\n\n' + printf 'Optional configuration example: [operator-config.yaml](https://github.com/%s/releases/download/%s/operator-config.yaml). ' \ + "${GITHUB_REPOSITORY}" "${TAG}" + printf 'Before applying it, check for an existing instance; see the [configuration guide](https://github.com/%s/blob/%s/docs/src/operations/configuration.md).\n' \ + "${GITHUB_REPOSITORY}" "${TAG}" + printf '\n' printf '### Helm\n\n' printf '```bash\n' printf 'helm install bootc-operator oci://ghcr.io/%s/charts/bootc-operator --version %s \\\n' \ @@ -181,4 +186,4 @@ jobs: --notes-file notes.md \ --generate-notes \ --draft \ - install.yaml + install.yaml operator-config.yaml diff --git a/.gitignore b/.gitignore index c1d5cf8b..50772344 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,8 @@ bin/ kubeconfig-* docs/book/ user-docs +/install.yaml +/operator-config.yaml chart/bootc-operator/crds/ chart/bootc-operator/charts/ chart/bootc-operator/Chart.lock diff --git a/Makefile b/Makefile index 2c9d43ce..c98c802e 100644 --- a/Makefile +++ b/Makefile @@ -200,11 +200,21 @@ buildimg: ## Build container image. -t $(IMG) . .PHONY: release-manifest -release-manifest: kustomize yq ## Build install manifest (override IMG to set the image reference). +release-manifest: kustomize yq ## Build install manifest and optional configuration asset (override IMG). "$(KUSTOMIZE)" build config/default | \ "$(YQ)" '(select(.kind == "Deployment") | .spec.template.spec.containers[] | select(.name == "manager")).image = "$(IMG)"' | \ "$(YQ)" '(select(.kind == "DaemonSet") | .spec.template.spec.containers[] | select(.name == "daemon")).image = "$(IMG)"' \ > install.yaml + cp config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml operator-config.yaml + +.PHONY: verify-release-manifest +verify-release-manifest: release-manifest ## Check the generated release assets. + @cmp -s config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml operator-config.yaml || \ + { echo "operator-config.yaml differs from the validated example"; exit 1; } + @test "$$("$(YQ)" ea '[.] | map(select(.kind == "CustomResourceDefinition" and .metadata.name == "bootcoperatorconfigs.node.bootc.dev")) | length' install.yaml)" -eq 1 || \ + { echo "install.yaml must contain the BootcOperatorConfig CRD exactly once"; exit 1; } + @test "$$("$(YQ)" ea '[.] | map(select(.kind == "BootcOperatorConfig")) | length' install.yaml)" -eq 0 || \ + { echo "install.yaml must not create an administrator-owned BootcOperatorConfig"; exit 1; } .PHONY: build-update-image build-update-image: ## Build derived node images for update testing and push to bink registry. @@ -281,7 +291,9 @@ start-bink: seed-node-image ## Start a bink cluster (idempotent). kubectl --kubeconfig $(KUBECONFIG_BINK) wait --for=condition=Ready node/controller --timeout=5m .PHONY: deploy-bink -deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_TAG),push-released-operator-image) helm ## Deploy to a bink cluster (requires: buildimg). +deploy-bink: start-bink helm ## Deploy to a bink cluster (requires: buildimg). + $(MAKE) build-update-image + $(if $(RELEASED_OPERATOR_TAG),$(MAKE) push-released-operator-image) podman push --tls-verify=false $(IMG) localhost:5000/bootc-operator-e2e:latest helm dependency build ./chart/bootc-operator kubectl --kubeconfig $(KUBECONFIG_BINK) create namespace $(E2E_PULL_SECRET_NAMESPACE) \ @@ -294,10 +306,9 @@ deploy-bink: start-bink build-update-image $(if $(RELEASED_OPERATOR_TAG),push-re --set-string image.tag="$${img##*:}" \ --set-string image.pullPolicy=Always \ --set-json 'pullSecretNamespaces=["$(E2E_PULL_SECRET_NAMESPACE)"]' \ - --set-json 'controller.extraArgs=["--allow-insecure-registry","--tag-resolution-interval=10s"]' \ --set-string 'daemon.extraArgs[0]=--bootc-poll-interval=$(E2E_BOOTC_POLL_INTERVAL)' - kubectl --kubeconfig $(KUBECONFIG_BINK) -n bootc-operator rollout restart deployment/bootc-operator-controller - kubectl --kubeconfig $(KUBECONFIG_BINK) -n bootc-operator rollout status deployment/bootc-operator-controller --timeout=3m + KUBECONFIG="$(abspath $(KUBECONFIG_BINK))" \ + KUBECTL="$(KUBECTL)" ./hack/deploy-bink.sh .PHONY: gather-bink gather-bink: ## Gather diagnostic logs from the bink cluster. diff --git a/PROJECT b/PROJECT index 198623b1..5e64685f 100644 --- a/PROJECT +++ b/PROJECT @@ -26,4 +26,12 @@ resources: kind: BootcNode path: github.com/jlebon/bootc-operator/api/v1alpha1 version: v1alpha1 +- api: + crdVersion: v1 + namespaced: false + domain: bootc.dev + group: node + kind: BootcOperatorConfig + path: github.com/bootc-dev/bootc-operator/api/v1alpha1 + version: v1alpha1 version: "3" diff --git a/README.md b/README.md index 20c09df0..97bf1d4d 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,12 @@ kubectl apply -k https://github.com/bootc-dev/bootc-operator//config/default This creates the `bootc-operator` namespace and deploys the controller and daemon. The operator does nothing until you create a BootcNodePool. +To configure the controller and daemon with an optional +`BootcOperatorConfig` resource, see +[Configuring the operator](docs/src/operations/configuration.md). Releases +publish an editable `operator-config.yaml` example separately from +`install.yaml`. + > [!NOTE] > The operator namespace requires a [Pod Security Admission] exemption for the > `privileged` level. The daemon DaemonSet runs privileged to execute bootc diff --git a/api/v1alpha1/bootcoperatorconfig_types.go b/api/v1alpha1/bootcoperatorconfig_types.go new file mode 100644 index 00000000..d0773480 --- /dev/null +++ b/api/v1alpha1/bootcoperatorconfig_types.go @@ -0,0 +1,89 @@ +// SPDX-License-Identifier: Apache-2.0 + +package v1alpha1 + +import metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + +const ( + // DefaultTagResolutionPeriodSeconds is the default interval between resolving image tags. + DefaultTagResolutionPeriodSeconds int32 = 300 + + // DefaultStatusPollPeriodSeconds is the default interval between fallback bootc status polls. + DefaultStatusPollPeriodSeconds int32 = 300 +) + +// OperatorControllerConfig configures the operator controller at startup. +type OperatorControllerConfig struct { + // allowInsecureRegistry allows falling back to HTTP when accessing image + // registries. Enable this only for trusted registries that do not support TLS. + // +optional + AllowInsecureRegistry *bool `json:"allowInsecureRegistry,omitempty"` + + // tagResolutionPeriodSeconds is the interval in seconds between resolving + // image tags to digests. When omitted, use the CLI value or 300 seconds. + // +optional + // +kubebuilder:validation:Minimum=1 + TagResolutionPeriodSeconds *int32 `json:"tagResolutionPeriodSeconds,omitempty"` + + // pullSecretNamespaces lists extra namespaces whose Secrets may be referenced + // by pools. The operator namespace is always allowed. Omission preserves CLI + // settings; an empty list allows no extras. Administrators must grant Secret get/list/watch access + // in each extra namespace separately. Changes require a controller restart. + // +optional + // +listType=set + // +kubebuilder:validation:items:MaxLength=63 + // +kubebuilder:validation:items:Pattern=`^[a-z0-9]([-a-z0-9]*[a-z0-9])?$` + PullSecretNamespaces []string `json:"pullSecretNamespaces,omitzero"` +} + +// OperatorDaemonConfig configures each node daemon at startup. +type OperatorDaemonConfig struct { + // statusPollPeriodSeconds is the interval in seconds between bootc status + // polls when filesystem notifications are unavailable. When omitted, use the + // CLI value or 300 seconds. + // +optional + // +kubebuilder:validation:Minimum=1 + StatusPollPeriodSeconds *int32 `json:"statusPollPeriodSeconds,omitempty"` +} + +// BootcOperatorConfigSpec defines operator settings consumed at process startup. +// Restart the affected controller or daemon pods after changing these settings. +type BootcOperatorConfigSpec struct { + // controller configures the controller. Omitted settings use CLI values or built-in defaults. + // +optional + // +kubebuilder:default={} + Controller *OperatorControllerConfig `json:"controller,omitempty"` + + // daemon configures all node daemons. Omitted settings use CLI values or built-in defaults. + // +optional + // +kubebuilder:default={} + Daemon *OperatorDaemonConfig `json:"daemon,omitempty"` +} + +// +kubebuilder:object:root=true +// +kubebuilder:resource:scope=Cluster + +// BootcOperatorConfig configures the controller and node daemons in a cluster. +// The optional resource is administrator-owned and may have any valid name. +// The operator rejects multiple configurations at startup. Changes take effect +// when the affected pods restart; supplied fields take precedence over CLI flags. +type BootcOperatorConfig struct { + metav1.TypeMeta `json:",inline"` + + // metadata is standard object metadata. + // +optional + metav1.ObjectMeta `json:"metadata,omitzero"` + + // spec defines the configuration consumed when operator processes start. + // +required + Spec BootcOperatorConfigSpec `json:"spec"` +} + +// +kubebuilder:object:root=true + +// BootcOperatorConfigList contains a list of BootcOperatorConfig. +type BootcOperatorConfigList struct { + metav1.TypeMeta `json:",inline"` + metav1.ListMeta `json:"metadata,omitzero"` + Items []BootcOperatorConfig `json:"items"` +} diff --git a/api/v1alpha1/groupversion_info.go b/api/v1alpha1/groupversion_info.go index 2575b926..c5a1426d 100644 --- a/api/v1alpha1/groupversion_info.go +++ b/api/v1alpha1/groupversion_info.go @@ -30,6 +30,7 @@ func addKnownTypes(scheme *runtime.Scheme) error { scheme.AddKnownTypes(SchemeGroupVersion, &BootcNode{}, &BootcNodeList{}, &BootcNodePool{}, &BootcNodePoolList{}, + &BootcOperatorConfig{}, &BootcOperatorConfigList{}, ) metav1.AddToGroupVersion(scheme, SchemeGroupVersion) return nil diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index be71e825..7e07c66e 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -249,6 +249,89 @@ func (in *BootcNodeStatus) DeepCopy() *BootcNodeStatus { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BootcOperatorConfig) DeepCopyInto(out *BootcOperatorConfig) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ObjectMeta.DeepCopyInto(&out.ObjectMeta) + in.Spec.DeepCopyInto(&out.Spec) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BootcOperatorConfig. +func (in *BootcOperatorConfig) DeepCopy() *BootcOperatorConfig { + if in == nil { + return nil + } + out := new(BootcOperatorConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *BootcOperatorConfig) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BootcOperatorConfigList) DeepCopyInto(out *BootcOperatorConfigList) { + *out = *in + out.TypeMeta = in.TypeMeta + in.ListMeta.DeepCopyInto(&out.ListMeta) + if in.Items != nil { + in, out := &in.Items, &out.Items + *out = make([]BootcOperatorConfig, len(*in)) + for i := range *in { + (*in)[i].DeepCopyInto(&(*out)[i]) + } + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BootcOperatorConfigList. +func (in *BootcOperatorConfigList) DeepCopy() *BootcOperatorConfigList { + if in == nil { + return nil + } + out := new(BootcOperatorConfigList) + in.DeepCopyInto(out) + return out +} + +// DeepCopyObject is an autogenerated deepcopy function, copying the receiver, creating a new runtime.Object. +func (in *BootcOperatorConfigList) DeepCopyObject() runtime.Object { + if c := in.DeepCopy(); c != nil { + return c + } + return nil +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *BootcOperatorConfigSpec) DeepCopyInto(out *BootcOperatorConfigSpec) { + *out = *in + if in.Controller != nil { + in, out := &in.Controller, &out.Controller + *out = new(OperatorControllerConfig) + (*in).DeepCopyInto(*out) + } + if in.Daemon != nil { + in, out := &in.Daemon, &out.Daemon + *out = new(OperatorDaemonConfig) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new BootcOperatorConfigSpec. +func (in *BootcOperatorConfigSpec) DeepCopy() *BootcOperatorConfigSpec { + if in == nil { + return nil + } + out := new(BootcOperatorConfigSpec) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *DisruptionSpec) DeepCopyInto(out *DisruptionSpec) { *out = *in @@ -298,6 +381,56 @@ func (in *ImageSpec) DeepCopy() *ImageSpec { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OperatorControllerConfig) DeepCopyInto(out *OperatorControllerConfig) { + *out = *in + if in.AllowInsecureRegistry != nil { + in, out := &in.AllowInsecureRegistry, &out.AllowInsecureRegistry + *out = new(bool) + **out = **in + } + if in.TagResolutionPeriodSeconds != nil { + in, out := &in.TagResolutionPeriodSeconds, &out.TagResolutionPeriodSeconds + *out = new(int32) + **out = **in + } + if in.PullSecretNamespaces != nil { + in, out := &in.PullSecretNamespaces, &out.PullSecretNamespaces + *out = make([]string, len(*in)) + copy(*out, *in) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorControllerConfig. +func (in *OperatorControllerConfig) DeepCopy() *OperatorControllerConfig { + if in == nil { + return nil + } + out := new(OperatorControllerConfig) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *OperatorDaemonConfig) DeepCopyInto(out *OperatorDaemonConfig) { + *out = *in + if in.StatusPollPeriodSeconds != nil { + in, out := &in.StatusPollPeriodSeconds, &out.StatusPollPeriodSeconds + *out = new(int32) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new OperatorDaemonConfig. +func (in *OperatorDaemonConfig) DeepCopy() *OperatorDaemonConfig { + if in == nil { + return nil + } + out := new(OperatorDaemonConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PullSecretRef) DeepCopyInto(out *PullSecretRef) { *out = *in diff --git a/cmd/controller/main.go b/cmd/controller/main.go index 62308d9a..e1ffd5f0 100644 --- a/cmd/controller/main.go +++ b/cmd/controller/main.go @@ -19,6 +19,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/log/zap" bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" + "github.com/bootc-dev/bootc-operator/internal/config" "github.com/bootc-dev/bootc-operator/internal/controller" "github.com/bootc-dev/bootc-operator/internal/registry" "github.com/bootc-dev/bootc-operator/internal/version" @@ -61,19 +62,19 @@ func main() { ) flag.DurationVar( &tagResolutionInterval, - "tag-resolution-interval", - 5*time.Minute, + config.TagResolutionIntervalFlag, + time.Duration(bootcv1alpha1.DefaultTagResolutionPeriodSeconds)*time.Second, "How often to re-resolve tag-based image refs.", ) flag.BoolVar( &allowInsecureRegistry, - "allow-insecure-registry", + config.AllowInsecureRegistryFlag, false, "Allow falling back to HTTP when resolving tag-based image refs against registries that do not serve TLS.", ) flag.StringVar( &pullSecretNamespacesFlag, - "pull-secret-namespaces", + config.PullSecretNamespacesFlag, "", "Comma-separated extra namespaces that pools may reference pull secrets from. "+ "The operator namespace is always allowed.", @@ -91,6 +92,7 @@ func main() { fmt.Println(version.String()) return } + explicit := config.ExplicitFlags(flag.CommandLine) ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -104,23 +106,43 @@ func main() { version.GitCommit, ) + ctx := ctrl.SetupSignalHandler() + restConfig := ctrl.GetConfigOrDie() + operatorConfig, err := config.Load(ctx, restConfig, scheme) + if err != nil { + setupLog.Error(err, "Failed to load operator configuration") + os.Exit(1) + } + if err := config.ApplyToFlags(flag.CommandLine, operatorConfig); err != nil { + setupLog.Error(err, "Failed to apply operator configuration") + os.Exit(1) + } operatorNamespace := detectNamespace() pullSecretNamespaces, err := controller.ParsePullSecretNamespaces( pullSecretNamespacesFlag, operatorNamespace, ) if err != nil { - setupLog.Error(err, "Invalid --pull-secret-namespaces") + setupLog.Error(err, "Invalid pull secret namespace configuration") os.Exit(1) } if len(pullSecretNamespaces) == 0 { setupLog.Error(nil, "No pull secret namespaces configured and operator namespace unknown; "+ - "set --pull-secret-namespaces or POD_NAMESPACE") + "set spec.controller.pullSecretNamespaces, --pull-secret-namespaces, or POD_NAMESPACE") os.Exit(1) } setupLog.Info("Pull secret namespaces", "namespaces", pullSecretNamespaces) - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + config.LogSource( + setupLog, + operatorConfig, + explicit, + config.AllowInsecureRegistryFlag, + config.TagResolutionIntervalFlag, + config.PullSecretNamespacesFlag, + ) + + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, HealthProbeBindAddress: probeAddr, LeaderElection: enableLeaderElection, @@ -165,7 +187,7 @@ func main() { } setupLog.Info("Starting manager") - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "Failed to run manager") os.Exit(1) } diff --git a/cmd/daemon/main.go b/cmd/daemon/main.go index c5fb680b..dcdd4868 100644 --- a/cmd/daemon/main.go +++ b/cmd/daemon/main.go @@ -20,6 +20,7 @@ import ( bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" "github.com/bootc-dev/bootc-operator/internal/bootc" + "github.com/bootc-dev/bootc-operator/internal/config" "github.com/bootc-dev/bootc-operator/internal/daemon" "github.com/bootc-dev/bootc-operator/internal/version" ) @@ -39,7 +40,7 @@ func main() { var showVersion bool flag.DurationVar( &pollInterval, - "bootc-poll-interval", + config.BootcPollIntervalFlag, daemon.DefaultPollInterval, "Interval for polling bootc status as a fallback to fsnotify", ) @@ -54,6 +55,7 @@ func main() { fmt.Println(version.String()) return } + explicit := config.ExplicitFlags(flag.CommandLine) ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) @@ -76,7 +78,20 @@ func main() { os.Exit(1) } - mgr, err := ctrl.NewManager(ctrl.GetConfigOrDie(), ctrl.Options{ + ctx := ctrl.SetupSignalHandler() + restConfig := ctrl.GetConfigOrDie() + operatorConfig, err := config.Load(ctx, restConfig, scheme) + if err != nil { + setupLog.Error(err, "Failed to load operator configuration") + os.Exit(1) + } + if err := config.ApplyToFlags(flag.CommandLine, operatorConfig); err != nil { + setupLog.Error(err, "Failed to apply operator configuration") + os.Exit(1) + } + config.LogSource(setupLog, operatorConfig, explicit, config.BootcPollIntervalFlag) + + mgr, err := ctrl.NewManager(restConfig, ctrl.Options{ Scheme: scheme, // Only cache the BootcNode object for this node to avoid unnecessary watches. Cache: cache.Options{ @@ -121,7 +136,7 @@ func main() { } setupLog.Info("Starting daemon", "node", nodeName, "pollInterval", pollInterval) - if err := mgr.Start(ctrl.SetupSignalHandler()); err != nil { + if err := mgr.Start(ctx); err != nil { setupLog.Error(err, "Failed to run daemon") os.Exit(1) } diff --git a/config/bink/operator-config.yaml b/config/bink/operator-config.yaml new file mode 100644 index 00000000..48c2136c --- /dev/null +++ b/config/bink/operator-config.yaml @@ -0,0 +1,8 @@ +apiVersion: node.bootc.dev/v1alpha1 +kind: BootcOperatorConfig +metadata: + name: cluster +spec: + controller: + allowInsecureRegistry: true + tagResolutionPeriodSeconds: 10 diff --git a/config/crd/bases/node.bootc.dev_bootcoperatorconfigs.yaml b/config/crd/bases/node.bootc.dev_bootcoperatorconfigs.yaml new file mode 100644 index 00000000..91078d4c --- /dev/null +++ b/config/crd/bases/node.bootc.dev_bootcoperatorconfigs.yaml @@ -0,0 +1,96 @@ +--- +apiVersion: apiextensions.k8s.io/v1 +kind: CustomResourceDefinition +metadata: + annotations: + controller-gen.kubebuilder.io/version: v0.20.1 + name: bootcoperatorconfigs.node.bootc.dev +spec: + group: node.bootc.dev + names: + kind: BootcOperatorConfig + listKind: BootcOperatorConfigList + plural: bootcoperatorconfigs + singular: bootcoperatorconfig + scope: Cluster + versions: + - name: v1alpha1 + schema: + openAPIV3Schema: + description: |- + BootcOperatorConfig configures the controller and node daemons in a cluster. + The optional resource is administrator-owned and may have any valid name. + The operator rejects multiple configurations at startup. Changes take effect + when the affected pods restart; supplied fields take precedence over CLI flags. + properties: + apiVersion: + description: |- + APIVersion defines the versioned schema of this representation of an object. + Servers should convert recognized schemas to the latest internal value, and + may reject unrecognized values. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources + type: string + kind: + description: |- + Kind is a string value representing the REST resource this object represents. + Servers may infer this from the endpoint the client submits requests to. + Cannot be updated. + In CamelCase. + More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds + type: string + metadata: + type: object + spec: + description: spec defines the configuration consumed when operator processes + start. + properties: + controller: + default: {} + description: controller configures the controller. Omitted settings + use CLI values or built-in defaults. + properties: + allowInsecureRegistry: + description: |- + allowInsecureRegistry allows falling back to HTTP when accessing image + registries. Enable this only for trusted registries that do not support TLS. + type: boolean + pullSecretNamespaces: + description: |- + pullSecretNamespaces lists extra namespaces whose Secrets may be referenced + by pools. The operator namespace is always allowed. Omission preserves CLI + settings; an empty list allows no extras. Administrators must grant Secret get/list/watch access + in each extra namespace separately. Changes require a controller restart. + items: + maxLength: 63 + pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?$ + type: string + type: array + x-kubernetes-list-type: set + tagResolutionPeriodSeconds: + description: |- + tagResolutionPeriodSeconds is the interval in seconds between resolving + image tags to digests. When omitted, use the CLI value or 300 seconds. + format: int32 + minimum: 1 + type: integer + type: object + daemon: + default: {} + description: daemon configures all node daemons. Omitted settings + use CLI values or built-in defaults. + properties: + statusPollPeriodSeconds: + description: |- + statusPollPeriodSeconds is the interval in seconds between bootc status + polls when filesystem notifications are unavailable. When omitted, use the + CLI value or 300 seconds. + format: int32 + minimum: 1 + type: integer + type: object + type: object + required: + - spec + type: object + served: true + storage: true diff --git a/config/crd/kustomization.yaml b/config/crd/kustomization.yaml index 24041517..7230f693 100644 --- a/config/crd/kustomization.yaml +++ b/config/crd/kustomization.yaml @@ -1,4 +1,5 @@ resources: - bases/node.bootc.dev_bootcnodepools.yaml - bases/node.bootc.dev_bootcnodes.yaml +- bases/node.bootc.dev_bootcoperatorconfigs.yaml # +kubebuilder:scaffold:crdkustomizeresource diff --git a/config/rbac/daemon_role.yaml b/config/rbac/daemon_role.yaml index f026a8eb..16dcdd4c 100644 --- a/config/rbac/daemon_role.yaml +++ b/config/rbac/daemon_role.yaml @@ -6,6 +6,12 @@ metadata: app.kubernetes.io/managed-by: kustomize name: daemon-role rules: +- apiGroups: + - node.bootc.dev + resources: + - bootcoperatorconfigs + verbs: + - list - apiGroups: - "" resources: diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 50e4ed03..9396faf9 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -83,6 +83,12 @@ rules: - bootcnodes/status verbs: - get +- apiGroups: + - node.bootc.dev + resources: + - bootcoperatorconfigs + verbs: + - list --- apiVersion: rbac.authorization.k8s.io/v1 kind: Role diff --git a/config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml b/config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml new file mode 100644 index 00000000..c226059e --- /dev/null +++ b/config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml @@ -0,0 +1,10 @@ +apiVersion: node.bootc.dev/v1alpha1 +kind: BootcOperatorConfig +metadata: + name: cluster +spec: + controller: + allowInsecureRegistry: false + tagResolutionPeriodSeconds: 300 + daemon: + statusPollPeriodSeconds: 300 diff --git a/docs/src/SUMMARY.md b/docs/src/SUMMARY.md index 1eb8249d..675afd62 100644 --- a/docs/src/SUMMARY.md +++ b/docs/src/SUMMARY.md @@ -7,6 +7,7 @@ - [Concepts](concepts.md) - [Helm Chart](helm.md) - [Operations](operations/index.md) + - [Configuring the operator](operations/configuration.md) - [Managing a pool](operations/pool.md) - [OS initialization taint](operations/os-not-ready-taint.md) - [Karpenter integration](operations/karpenter.md) diff --git a/docs/src/helm.md b/docs/src/helm.md index 91a73e49..154f4b19 100644 --- a/docs/src/helm.md +++ b/docs/src/helm.md @@ -26,27 +26,66 @@ helm install bootc-operator oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc- --version 0.1.0 --create-namespace --namespace bootc-operator ``` -Alternatively, install from a local checkout: +Alternatively, generate and install the chart from a local checkout: ```bash +make helm helm dependency build ./chart/bootc-operator helm install bootc-operator ./chart/bootc-operator \ --create-namespace --namespace bootc-operator ``` -To manage CRDs separately (e.g. with a GitOps tool), disable the -dependency and install the CRD chart on its own: - -```bash -helm install bootc-operator-crds oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator-crds -helm install bootc-operator oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator \ - --create-namespace --namespace bootc-operator --set crds.install=false -``` +The chart installs the configuration CRD and the operator's read permissions. +It does not create a `BootcOperatorConfig` instance. See +[Configuring the operator](operations/configuration.md) to add one. By default the chart uses the image `ghcr.io/bootc-dev/bootc-operator:` where `appVersion` is defined in `Chart.yaml`. +### Configure before first startup + +For custom settings on first startup, install the CRDs and operator as separate +Helm releases. This procedure is for a fresh installation; do not use it to +transfer CRDs from an existing combined release. + +1. Install the CRDs. Replace `X.Y.Z` with a published chart version containing + `BootcOperatorConfig`, and use that version for both charts: + + ```bash + CHART_VERSION=X.Y.Z + helm install bootc-operator-crds \ + oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator-crds \ + --version "$CHART_VERSION" --create-namespace --namespace bootc-operator + kubectl wait --for=condition=Established \ + crd/bootcoperatorconfigs.node.bootc.dev --timeout=1m + ``` + +2. Check for an existing configuration. If one exists, edit it instead of + creating another. Otherwise, prepare an `operator-config.yaml` using the + [configuration example](operations/configuration.md), set the desired + values, and apply it: + + ```bash + kubectl get bootcoperatorconfigs + kubectl apply -f operator-config.yaml + ``` + +3. Install the operator with its CRD dependency disabled, since the separate + release already owns the CRDs: + + ```bash + helm install bootc-operator \ + oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator \ + --version "$CHART_VERSION" --namespace bootc-operator --set crds.install=false + ``` + +The controller and daemon read the configuration on their first startup. +Later configuration edits still require restarting the affected workloads. If +the CR allows extra pull-secret namespaces, provision their +[namespaced permissions](operations/configuration.md#pull-secret-namespaces) +before installing the operator. + ## Custom image Override the repository and tag to use a different image: @@ -102,6 +141,32 @@ kubectl create secret generic my-pull-secret \ --type=kubernetes.io/dockerconfigjson ``` +## Pool pull secret namespaces + +`pullSecretNamespaces` configures extra namespaces for pool `pullSecretRef` +references. This is separate from `imagePullSecrets`, which authenticates pulling +the operator's own image. The release namespace is always allowed. + +Configure the namespace list and its permissions as follows: + +- **Helm:** set `pullSecretNamespaces` to existing namespace names. Helm creates + their Secret permissions and supplies a CLI value used when the CR field is omitted. +- **BootcOperatorConfig:** its namespace list takes precedence over the Helm CLI + value. Grant access to every namespace in that list, either through matching + Helm values or the [RBAC example](operations/configuration.md#pull-secret-namespaces). + The CR does not create or remove permissions. + +For Helm-managed access: + +```bash +helm upgrade --install bootc-operator ./chart/bootc-operator \ + --namespace bootc-operator \ + --set-json 'pullSecretNamespaces=["team-a","team-b"]' +``` + +See [precedence](operations/configuration.md#precedence-and-startup-failures) +for CR overrides and CLI fallback, including restricting access to the operator namespace. + ## Upgrade ```bash @@ -112,6 +177,17 @@ helm upgrade bootc-operator oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc- Replace `` with the chart version (e.g. `0.2.0`). The CRD sub-chart is upgraded automatically. +If you installed CRDs separately, upgrade their release first, then upgrade the +operator with `--set crds.install=false`. Use the same target chart version for +both commands: + +```bash +helm upgrade bootc-operator-crds oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator-crds \ + --version --namespace bootc-operator +helm upgrade bootc-operator oci://ghcr.io/bootc-dev/bootc-operator/charts/bootc-operator \ + --version --namespace bootc-operator --set crds.install=false +``` + ## Uninstall ```bash @@ -119,7 +195,11 @@ helm uninstall bootc-operator --namespace bootc-operator ``` > **Warning**: uninstalling the chart also removes the CRDs and all -> their instances cluster-wide. +> their instances cluster-wide, including administrator-owned +> `BootcOperatorConfig` settings. This applies when the CRD dependency is enabled +> (the default). With separate releases, uninstalling only `bootc-operator` +> preserves the CRDs and configuration. Uninstalling `bootc-operator-crds` +> deletes the CRDs and all their instances. ## Regenerating the chart @@ -140,4 +220,6 @@ make helm | `image.tag` | `""` (uses `appVersion`) | Operator image tag | | `image.pullPolicy` | not set | Kubernetes image pull policy | | `imagePullSecrets` | `[]` | List of pull secret references | +| `pullSecretNamespaces` | `[]` | Extra pool pull-secret namespaces; non-empty values configure a CLI fallback and namespaced RBAC | | `controller.extraArgs` | `[]` | Extra arguments for the controller | +| `daemon.extraArgs` | `[]` | Extra arguments for the node daemon | diff --git a/docs/src/operations/configuration.md b/docs/src/operations/configuration.md new file mode 100644 index 00000000..8a1fc247 --- /dev/null +++ b/docs/src/operations/configuration.md @@ -0,0 +1,188 @@ +# Configuring the operator + +Configure the controller and daemon with a cluster-scoped +`BootcOperatorConfig`. Its typed `node.bootc.dev/v1alpha1` API keeps +administrator settings separate from installation manifests. Choose any valid +Kubernetes resource name; use at most one configuration in the cluster. + +For example: + +```yaml +apiVersion: node.bootc.dev/v1alpha1 +kind: BootcOperatorConfig +metadata: + name: cluster +spec: + controller: + tagResolutionPeriodSeconds: 60 + daemon: + statusPollPeriodSeconds: 60 +``` + +Installation does not create or overwrite your configuration. Without one, +the operator uses its defaults and command-line arguments. `cluster` is an +example name; the resource has no namespace. + +## Supported settings + +| Field under spec | Component | Built-in default | +| --- | --- | --- | +| `controller.allowInsecureRegistry` | Controller | `false` | +| `controller.pullSecretNamespaces` | Controller | `[]` (operator namespace only) | +| `controller.tagResolutionPeriodSeconds` | Controller | `300` (5 minutes) | +| `daemon.statusPollPeriodSeconds` | Daemon | `300` (5 minutes) | + +Period values are positive whole seconds. Invalid types and zero or negative +periods are rejected by the API server. Omitted fields, including an empty +`spec: {}`, use CLI values or built-in defaults. Either section may be omitted. +Use YAML booleans and integers directly, without string quotes. + +`allowInsecureRegistry` lets the controller's tag resolver fall back to HTTP +for registries without TLS. The bink development workflow enables it for its +local registry. Registry credentials continue to use each pool's +`pullSecretRef`. + +The daemon normally observes bootc status through filesystem notifications. +`statusPollPeriodSeconds` controls fallback polling when those notifications +are unavailable. + +## Pull secret namespaces + +Allow pools to use pull secrets from additional namespaces with +`controller.pullSecretNamespaces`. The operator's own namespace is always +allowed. Omit the field to use CLI settings, or set `[]` for no extras even +when CLI flags list namespaces. Use unique, valid namespace names. Add this +setting to your existing configuration: + +```yaml +spec: + controller: + pullSecretNamespaces: + - team-a +``` + +This setting does not grant permissions. Create the namespace and grant the +controller access to its Secrets before adding it. For a Helm release named +`bootc-operator` in namespace `bootc-operator`, apply this example in `team-a`: + +```yaml +apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: bootc-operator-pull-secrets + namespace: team-a +rules: + - apiGroups: [""] + resources: ["secrets"] + verbs: ["get", "list", "watch"] +--- +apiVersion: rbac.authorization.k8s.io/v1 +kind: RoleBinding +metadata: + name: bootc-operator-pull-secrets + namespace: team-a +roleRef: + apiGroup: rbac.authorization.k8s.io + kind: Role + name: bootc-operator-pull-secrets +subjects: + - kind: ServiceAccount + name: bootc-operator-controller + namespace: bootc-operator +``` + +Adjust the ServiceAccount name and namespace for your installation; +`install.yaml` uses `bootc-operator-controller-manager` instead. +For Helm-managed permissions, see +[Helm configuration](../helm.md#pool-pull-secret-namespaces). + +After granting permissions and applying the configuration, [restart the +controller](#applying-configuration). To remove access, remove the namespace +from the CR list (or Helm/CLI setting when the CR field is omitted), then +restart before deleting its Role and RoleBinding. + +## Applying configuration + +To use custom settings on the first startup with Helm, follow +[Configure before first startup](../helm.md#configure-before-first-startup). + +After installing or upgrading the operator, check for an existing configuration: + +```shell +kubectl get bootcoperatorconfigs +``` + +If none exists, edit and apply the sample: + +```shell +kubectl apply -f config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml +kubectl get bootcoperatorconfig cluster -o yaml +``` + +The sample explicitly sets default values, which override matching CLI flags. +Edit it or remove fields you want to leave to CLI settings before applying it. +If a configuration already exists, edit that resource instead of creating another. + +Releases also provide a separate, optional `operator-config.yaml` asset with +the same default values. On a released installation, replace `vX.Y.Z` with +the installed release tag, then download and edit the file before applying it: + +```shell +curl -fL -o operator-config.yaml \ + https://github.com/bootc-dev/bootc-operator/releases/download/vX.Y.Z/operator-config.yaml +# Edit operator-config.yaml, then: +kubectl apply -f operator-config.yaml +``` + +The configuration is read once when each controller or daemon process starts. +Editing the resource does not restart pods or change running processes. +Restart the workload whose settings changed. The example above changes both: + +```shell +kubectl -n bootc-operator rollout restart deployment/bootc-operator-controller-manager +kubectl -n bootc-operator rollout status deployment/bootc-operator-controller-manager --timeout=3m +kubectl -n bootc-operator rollout restart daemonset/bootc-operator-daemon +kubectl -n bootc-operator rollout status daemonset/bootc-operator-daemon --timeout=3m +``` + +For Helm installations, use `deployment/bootc-operator-controller` instead of +`deployment/bootc-operator-controller-manager` in the restart and status commands. +Use the workload names and namespace from your installation if customized. +Daemon pods run only on nodes selected by a pool. New pods read the current +configuration automatically. Restarting the controller alone does not +reconfigure existing daemon pods. + +## Precedence and startup failures + +For each supported setting, the value is selected in this order: + +1. A field explicitly set in `BootcOperatorConfig`. +2. An explicitly supplied command-line flag when the CR field is omitted. +3. The built-in default. + +Explicit `false` and `[]` in the CR override CLI values. For example, +`pullSecretNamespaces: []` permits only the operator's namespace, even if +Helm or CLI flags list extra namespaces. Omitted duration fields retain CLI +duration syntax and precision, including fractional seconds. + +More than one configuration, a missing CRD, denied API access, or a connection +failure stops startup with an error. Remove extra configurations, install the +matching CRD and permissions, or restore API connectivity before restarting. +A running process keeps its loaded settings during a later API outage. + +## Reverting and upgrading + +Restore a field's previous value, or remove it to use the CLI value or built-in +default, then restart the affected workload. To remove all overrides from this resource: + +```shell +kubectl delete bootcoperatorconfig cluster +``` + +Replace `cluster` with the resource's actual name if you used a different one. +Then restart the affected controller and daemon pods as above. Deletion alone +does not change running processes. Explicit command-line flags still apply. + +Reapplying installation manifests during an ordinary upgrade preserves the +configuration resource. For Helm installation and removal behavior, see the +[Helm guide](../helm.md). diff --git a/go.mod b/go.mod index f99b633b..f56665ff 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/bootc-dev/bootc-operator -go 1.26.10 +go 1.26.11 require ( github.com/aws/aws-sdk-go-v2 v1.47.8 diff --git a/hack/deploy-bink.sh b/hack/deploy-bink.sh new file mode 100755 index 00000000..f75d8670 --- /dev/null +++ b/hack/deploy-bink.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# Configure the operator after make deploy-bink installs its Helm chart. +set -euo pipefail + +: "${KUBECONFIG:?must be set}" +: "${KUBECTL:=kubectl}" +cd "$(dirname "${BASH_SOURCE[0]}")/.." + +"$KUBECTL" --kubeconfig "$KUBECONFIG" wait --for=condition=Established \ + crd/bootcoperatorconfigs.node.bootc.dev --timeout=1m +"$KUBECTL" --kubeconfig "$KUBECONFIG" apply -f config/bink/operator-config.yaml +"$KUBECTL" --kubeconfig "$KUBECONFIG" -n bootc-operator rollout restart \ + deployment/bootc-operator-controller +"$KUBECTL" --kubeconfig "$KUBECONFIG" -n bootc-operator rollout status \ + deployment/bootc-operator-controller --timeout=3m diff --git a/hack/gather-logs.sh b/hack/gather-logs.sh index 474d1318..8d726e13 100755 --- a/hack/gather-logs.sh +++ b/hack/gather-logs.sh @@ -37,6 +37,7 @@ run "k-describe-pods.txt" kubectl describe pods -n bootc-operator run "k-get-deployment.yaml" kubectl get deployment -n bootc-operator -o yaml run "k-describe-bootcnodepools.txt" kubectl describe bootcnodepools run "k-describe-bootcnodes.txt" kubectl describe bootcnodes +run "k-get-operator-config.yaml" kubectl get bootcoperatorconfigs.node.bootc.dev -o yaml run "k-get-events.txt" kubectl get events -n bootc-operator --sort-by=.lastTimestamp # Pod logs diff --git a/hack/sync-helm.sh b/hack/sync-helm.sh index b5060f2f..746f6bbd 100755 --- a/hack/sync-helm.sh +++ b/hack/sync-helm.sh @@ -16,6 +16,8 @@ command -v "$YQ" >/dev/null 2>&1 || { echo "error: yq required (run: make yq)" > # --- CRDs (separate chart) --- mkdir -p "$CRD_CHART_DIR/templates" cp "$CONFIG_DIR"/crd/bases/*.yaml "$CRD_CHART_DIR/templates/" +# Remove generated CRDs from the old layout to avoid packaging them twice. +rm -rf -- "$CHART_DIR/crds" echo "Synced CRDs" # --- RBAC --- @@ -89,9 +91,11 @@ gen_workload() { gen_workload "$CONFIG_DIR/manager/manager.yaml" Deployment controller \ "$CHART_DIR/templates/controller-deployment.yaml" -# Inject --pull-secret-namespaces and controller.extraArgs into the args list +# Supply CLI namespace settings only when explicitly configured in Helm. sed -i '/^ image: {{ include "bootc-operator\.image" \. }}/i\ + {{- if .Values.pullSecretNamespaces }}\ - {{ printf "--pull-secret-namespaces=%s" (include "bootc-operator.pullSecretNamespaces" .) | quote }}\ + {{- end }}\ {{- range .Values.controller.extraArgs }}\ - {{ . | quote }}\ {{- end }}' "$CHART_DIR/templates/controller-deployment.yaml" diff --git a/hack/test-helm.sh b/hack/test-helm.sh index c77d6e87..30d84ac6 100755 --- a/hack/test-helm.sh +++ b/hack/test-helm.sh @@ -19,18 +19,18 @@ render() { "$HELM" template test "$CHART_DIR" --namespace "$NS" --kube-version 1.34.0 "$@" } -# check MANIFESTS FLAG_VALUE NAMESPACES... -# Asserts that the controller flag is FLAG_VALUE, that each namespace has +# check MANIFESTS FLAG_ARGUMENTS NAMESPACES... +# Asserts the namespace flag arguments in order (empty means absent), each namespace has # exactly one secret-reading Role bound to the controller ServiceAccount, # and that the controller ClusterRole does not grant access to secrets. check() { local manifests="$1" flag="$2"; shift 2 - local want got + local want got roles want=$(printf '%s\n' "$@" | sort) got=$(echo "$manifests" | "$YQ" -N 'select(.kind == "Deployment") | .spec.template.spec.containers[0].args[] | select(test("^--pull-secret-namespaces="))') - [ "$got" = "--pull-secret-namespaces=$flag" ] || fail "controller flag is [$got]" + [ "$got" = "$flag" ] || fail "controller flag is [$got]" roles=$(echo "$manifests" | "$YQ" -N 'select(.kind == "Role") | select(.rules | any_c(.resources | any_c(. == "secrets"))) | @@ -52,10 +52,17 @@ check() { "$HELM" dependency build "$CHART_DIR" >/dev/null -check "$(render)" "$NS" "$NS" +check "$(render)" "" "$NS" # Duplicates and the release namespace itself must not produce extra RBAC. check "$(render --set "pullSecretNamespaces={team-a,$NS,team-b,team-a}")" \ - "$NS,team-a,team-b" "$NS" team-a team-b + "--pull-secret-namespaces=$NS,team-a,team-b" "$NS" team-a team-b + +# Explicit extra arguments follow Helm-derived flags; CR fields still take precedence. +check "$(render --set-string 'controller.extraArgs[0]=--pull-secret-namespaces=')" \ + "--pull-secret-namespaces=" "$NS" +check "$(render --set 'pullSecretNamespaces={team-a}' \ + --set-string 'controller.extraArgs[0]=--pull-secret-namespaces=')" \ + "$(printf '%s\n' "--pull-secret-namespaces=$NS,team-a" "--pull-secret-namespaces=")" "$NS" team-a for bad in "pullSecretNamespaces=team-a" "pullSecretNamespaces={team-a,}" \ "pullSecretNamespaces={Team-A}"; do diff --git a/internal/config/config.go b/internal/config/config.go new file mode 100644 index 00000000..388cd5b9 --- /dev/null +++ b/internal/config/config.go @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package config loads and applies optional operator configuration at startup. +package config + +import ( + "flag" + "fmt" + "strconv" + "strings" + "time" + + "github.com/go-logr/logr" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +// Flag names shared by startup configuration and the component entry points. +const ( + AllowInsecureRegistryFlag = "allow-insecure-registry" + TagResolutionIntervalFlag = "tag-resolution-interval" + BootcPollIntervalFlag = "bootc-poll-interval" + PullSecretNamespacesFlag = "pull-secret-namespaces" +) + +// ExplicitFlags snapshots flags supplied on the command line, including false +// boolean values. Call it immediately after parsing, before applying configuration. +func ExplicitFlags(fs *flag.FlagSet) map[string]bool { + explicit := make(map[string]bool) + fs.Visit(func(f *flag.Flag) { explicit[f.Name] = true }) + return explicit +} + +// ApplyToFlags applies fields supported by this process to parsed flag values. +// Supplied resource fields override CLI values; omitted fields retain CLI values +// or built-in defaults. Setting Flag.Value directly does not mark CR values as CLI flags. +// cfg must have been validated by Load. +func ApplyToFlags(fs *flag.FlagSet, cfg *bootcv1alpha1.BootcOperatorConfig) error { + if cfg == nil { + return nil + } + set := func(name, value string) error { + f := fs.Lookup(name) + if f == nil { + return nil // The other process may not register this flag. + } + if err := f.Value.Set(value); err != nil { + return fmt.Errorf("apply BootcOperatorConfig/%s to --%s: %w", cfg.Name, name, err) + } + return nil + } + if c := cfg.Spec.Controller; c != nil { + if c.PullSecretNamespaces != nil { + if err := set( + PullSecretNamespacesFlag, + strings.Join(c.PullSecretNamespaces, ","), + ); err != nil { + return err + } + } + if c.AllowInsecureRegistry != nil { + if err := set( + AllowInsecureRegistryFlag, + strconv.FormatBool(*c.AllowInsecureRegistry), + ); err != nil { + return err + } + } + if c.TagResolutionPeriodSeconds != nil { + period := time.Duration(*c.TagResolutionPeriodSeconds) * time.Second + if err := set(TagResolutionIntervalFlag, period.String()); err != nil { + return err + } + } + } + if d := cfg.Spec.Daemon; d != nil && d.StatusPollPeriodSeconds != nil { + period := time.Duration(*d.StatusPollPeriodSeconds) * time.Second + if err := set(BootcPollIntervalFlag, period.String()); err != nil { + return err + } + } + return nil +} + +// LogSource records the selected resource and supplied CLI flags without logging +// credentials or bootstrap flags. Supported flags are supplied by the component. +func LogSource( + log logr.Logger, + cfg *bootcv1alpha1.BootcOperatorConfig, + explicit map[string]bool, + flags ...string, +) { + overrides := make([]string, 0, len(flags)) + for _, name := range flags { + if explicit[name] { + overrides = append(overrides, name) + } + } + if cfg == nil { + log.Info( + "Loaded operator configuration", + "source", + "defaults", + "suppliedCLIFlags", + overrides, + ) + return + } + log.Info("Loaded operator configuration", "source", "BootcOperatorConfig/"+cfg.Name, + "uid", cfg.UID, "generation", cfg.Generation, "suppliedCLIFlags", overrides) +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go new file mode 100644 index 00000000..a4004170 --- /dev/null +++ b/internal/config/config_test.go @@ -0,0 +1,173 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "errors" + "flag" + "math" + "testing" + "time" + + . "github.com/onsi/gomega" + "k8s.io/utils/ptr" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +func TestApplyToFlags(t *testing.T) { + for _, tt := range []struct { + name string + controller *bootcv1alpha1.OperatorControllerConfig + daemon *bootcv1alpha1.OperatorDaemonConfig + args []string + want bool + wantTag time.Duration + wantPoll time.Duration + wantNamespaces string + }{ + {name: "absent instance", wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute}, + { + name: "omitted CR fields retain CLI values", + controller: &bootcv1alpha1.OperatorControllerConfig{}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{}, + args: []string{"--allow-insecure-registry", "--tag-resolution-interval=1500ms", "--bootc-poll-interval=500ms", "--pull-secret-namespaces=team-a"}, + want: true, wantTag: 1500 * time.Millisecond, wantPoll: 500 * time.Millisecond, wantNamespaces: "team-a", + }, + { + name: "configured namespaces", + controller: &bootcv1alpha1.OperatorControllerConfig{PullSecretNamespaces: []string{"team-a", "team-b"}}, + wantNamespaces: "team-a,team-b", wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "empty CR namespaces override CLI", + args: []string{"--pull-secret-namespaces=team-b"}, + controller: &bootcv1alpha1.OperatorControllerConfig{PullSecretNamespaces: []string{}}, + wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "CR namespaces override CLI", + controller: &bootcv1alpha1.OperatorControllerConfig{PullSecretNamespaces: []string{"team-a"}}, + args: []string{"--pull-secret-namespaces=team-b"}, + wantNamespaces: "team-a", wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "CR namespaces override empty CLI", + controller: &bootcv1alpha1.OperatorControllerConfig{PullSecretNamespaces: []string{"team-a"}}, + args: []string{"--pull-secret-namespaces="}, + wantNamespaces: "team-a", + wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "empty sections", controller: &bootcv1alpha1.OperatorControllerConfig{}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{}, wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "partial controller", controller: &bootcv1alpha1.OperatorControllerConfig{AllowInsecureRegistry: ptr.To(true)}, + want: true, wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "configured periods", + controller: &bootcv1alpha1.OperatorControllerConfig{TagResolutionPeriodSeconds: ptr.To(int32(10))}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(17))}, + wantTag: 10 * time.Second, wantPoll: 17 * time.Second, + }, + { + name: "maximum periods do not overflow", + controller: &bootcv1alpha1.OperatorControllerConfig{TagResolutionPeriodSeconds: ptr.To(int32(math.MaxInt32))}, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(math.MaxInt32))}, + wantTag: time.Duration(math.MaxInt32) * time.Second, + wantPoll: time.Duration(math.MaxInt32) * time.Second, + }, + { + name: "CR overrides explicit false and fractional durations", + controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), TagResolutionPeriodSeconds: ptr.To(int32(10)), + }, + daemon: &bootcv1alpha1.OperatorDaemonConfig{StatusPollPeriodSeconds: ptr.To(int32(17))}, + args: []string{ + "--allow-insecure-registry=false", + "--tag-resolution-interval=1500ms", + "--bootc-poll-interval=500ms", + }, + want: true, wantTag: 10 * time.Second, wantPoll: 17 * time.Second, + }, + { + name: "CR false overrides explicit true", + controller: &bootcv1alpha1.OperatorControllerConfig{AllowInsecureRegistry: ptr.To(false)}, + args: []string{"--allow-insecure-registry"}, + wantTag: 5 * time.Minute, wantPoll: 5 * time.Minute, + }, + { + name: "omitted CR fields retain zero and negative CLI durations", + controller: &bootcv1alpha1.OperatorControllerConfig{}, + args: []string{"--tag-resolution-interval=0", "--bootc-poll-interval=-1s"}, + wantPoll: -time.Second, + }, + } { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + var allowInsecure bool + namespaces := fs.String(PullSecretNamespacesFlag, "", "") + var tagInterval, pollInterval time.Duration + fs.BoolVar(&allowInsecure, AllowInsecureRegistryFlag, false, "") + fs.DurationVar(&tagInterval, TagResolutionIntervalFlag, 5*time.Minute, "") + fs.DurationVar(&pollInterval, BootcPollIntervalFlag, 5*time.Minute, "") + g.Expect(fs.Parse(tt.args)).To(Succeed()) + explicit := ExplicitFlags(fs) + var cfg *bootcv1alpha1.BootcOperatorConfig + if tt.controller != nil || tt.daemon != nil { + cfg = &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: tt.controller, + Daemon: tt.daemon, + }, + } + } + g.Expect(ApplyToFlags(fs, cfg)).To(Succeed()) + g.Expect(allowInsecure).To(Equal(tt.want)) + g.Expect(*namespaces).To(Equal(tt.wantNamespaces)) + g.Expect(tagInterval).To(Equal(tt.wantTag)) + g.Expect(pollInterval).To(Equal(tt.wantPoll)) + g.Expect(ExplicitFlags(fs)). + To(Equal(explicit), "configuration must not mark flags as explicit") + }) + } +} + +func TestApplyToFlagsSkipsOtherComponent(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + var pollInterval time.Duration + fs.DurationVar(&pollInterval, BootcPollIntervalFlag, 5*time.Minute, "") + cfg := &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), + PullSecretNamespaces: []string{"team-a"}, + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(int32(17)), + }, + }, + } + g.Expect(ApplyToFlags(fs, cfg)).To(Succeed()) + g.Expect(pollInterval).To(Equal(17 * time.Second)) +} + +func TestApplyToFlagsReportsSetterError(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + fs.Func(AllowInsecureRegistryFlag, "", func(string) error { return errors.New("rejected") }) + cfg := &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), + PullSecretNamespaces: []string{"team-a"}, + }, + }, + } + g.Expect(ApplyToFlags(fs, cfg)). + To(MatchError(ContainSubstring("--allow-insecure-registry: rejected"))) +} diff --git a/internal/config/load.go b/internal/config/load.go new file mode 100644 index 00000000..bfa7e9f6 --- /dev/null +++ b/internal/config/load.go @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "context" + "fmt" + "sort" + "strings" + "time" + + "k8s.io/apimachinery/pkg/api/meta" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + utilnet "k8s.io/apimachinery/pkg/util/net" + "k8s.io/apimachinery/pkg/util/validation" + "k8s.io/client-go/rest" + "sigs.k8s.io/controller-runtime/pkg/client" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +// Bound the bootstrap read without imposing a timeout on the manager's watches. +const startupTimeout = 10 * time.Second + +// Load lists configurations directly from the API before caches start. An empty +// list permits defaults; installation, permission, and network failures do not. +func Load( + ctx context.Context, + restConfig *rest.Config, + scheme *runtime.Scheme, +) (*bootcv1alpha1.BootcOperatorConfig, error) { + return load(ctx, restConfig, scheme, startupTimeout) +} + +func load( + ctx context.Context, + restConfig *rest.Config, + scheme *runtime.Scheme, + timeout time.Duration, +) (*bootcv1alpha1.BootcOperatorConfig, error) { + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + bootstrap := rest.CopyConfig(restConfig) + bootstrap.Timeout = timeout + // This client only lists our known resource, so API discovery is unnecessary. + gv := bootcv1alpha1.GroupVersion + mapper := meta.NewDefaultRESTMapper([]schema.GroupVersion{gv}) + mapper.AddSpecific( + gv.WithKind("BootcOperatorConfig"), + gv.WithResource( + "bootcoperatorconfigs", + ), + gv.WithResource("bootcoperatorconfig"), + meta.RESTScopeRoot, + ) + httpClient, err := rest.HTTPClientFor(bootstrap) + if err != nil { + return nil, fmt.Errorf("create HTTP client for BootcOperatorConfig: %w", err) + } + defer utilnet.CloseIdleConnectionsFor(httpClient.Transport) + c, err := client.New(bootstrap, client.Options{ + Scheme: scheme, Mapper: mapper, HTTPClient: httpClient, + }) + if err != nil { + return nil, fmt.Errorf("create client for BootcOperatorConfig: %w", err) + } + var configs bootcv1alpha1.BootcOperatorConfigList + if err := c.List(ctx, &configs); err != nil { + return nil, fmt.Errorf( + "list BootcOperatorConfig: ensure the bootcoperatorconfigs CRD is installed, "+ + "the API server is reachable, and this service account can list bootcoperatorconfigs: %w", + err, + ) + } + if len(configs.Items) == 0 { + return nil, nil + } + if len(configs.Items) > 1 { + names := make([]string, 0, len(configs.Items)) + for _, cfg := range configs.Items { + names = append(names, cfg.Name) + } + sort.Strings(names) + return nil, fmt.Errorf("found multiple BootcOperatorConfig resources (%s); keep only one", + strings.Join(names, ", ")) + } + cfg := &configs.Items[0] + if err := validate(cfg); err != nil { + return nil, err + } + return cfg, nil +} + +func validate(cfg *bootcv1alpha1.BootcOperatorConfig) error { + if cfg.Name == "" { + return fmt.Errorf("BootcOperatorConfig list returned an object without a name") + } + if c := cfg.Spec.Controller; c != nil && c.TagResolutionPeriodSeconds != nil && + *c.TagResolutionPeriodSeconds < 1 { + return fmt.Errorf( + "BootcOperatorConfig/%s spec.controller.tagResolutionPeriodSeconds must be at least 1", + cfg.Name, + ) + } + if c := cfg.Spec.Controller; c != nil { + seen := make(map[string]bool, len(c.PullSecretNamespaces)) + for i, namespace := range c.PullSecretNamespaces { + field := fmt.Sprintf( + "BootcOperatorConfig/%s spec.controller.pullSecretNamespaces[%d]", + cfg.Name, + i, + ) + if errs := validation.IsDNS1123Label(namespace); len(errs) > 0 { + return fmt.Errorf( + "%s: invalid namespace %q: %s", + field, + namespace, + strings.Join(errs, "; "), + ) + } + if seen[namespace] { + return fmt.Errorf("%s: duplicate namespace %q", field, namespace) + } + seen[namespace] = true + } + } + if d := cfg.Spec.Daemon; d != nil && d.StatusPollPeriodSeconds != nil && + *d.StatusPollPeriodSeconds < 1 { + return fmt.Errorf( + "BootcOperatorConfig/%s spec.daemon.statusPollPeriodSeconds must be at least 1", + cfg.Name, + ) + } + return nil +} diff --git a/internal/config/load_test.go b/internal/config/load_test.go new file mode 100644 index 00000000..2bb78997 --- /dev/null +++ b/internal/config/load_test.go @@ -0,0 +1,252 @@ +// SPDX-License-Identifier: Apache-2.0 + +package config + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "strings" + "testing" + "time" + + . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/rest" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" +) + +const configPath = "/apis/node.bootc.dev/v1alpha1/bootcoperatorconfigs" + +func TestLoad(t *testing.T) { + resource := schema.GroupResource{Group: "node.bootc.dev", Resource: "bootcoperatorconfigs"} + for _, tt := range []struct { + name string + status *apierrors.StatusError + body string + code int + wantNil bool + wantError string + }{ + {name: "one custom-named object"}, + {name: "no objects", body: `{"items":[]}`, wantNil: true}, + { + name: "multiple objects", body: `{"items":[{"metadata":{"name":"z-config"}},` + + `{"metadata":{"name":"a-config"}}]}`, + wantError: "a-config, z-config); keep only one", + }, + { + name: "missing CRD", status: apierrors.NewNotFound(resource, ""), + wantError: "ensure the bootcoperatorconfigs CRD is installed", + }, + { + name: "plain HTTP 404", code: 404, body: "not found", + wantError: "ensure the bootcoperatorconfigs CRD is installed", + }, + { + name: "forbidden", status: apierrors.NewForbidden(resource, "", errors.New("access denied")), + wantError: "this service account can list bootcoperatorconfigs", + }, + { + name: "unauthorized", status: apierrors.NewUnauthorized("authentication failed"), + wantError: "authentication failed", + }, + {name: "malformed response", body: `{`, wantError: "unexpected end"}, + { + name: "zero daemon period", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"daemon":{"statusPollPeriodSeconds":0}}}]}`, + wantError: "BootcOperatorConfig/custom-config spec.daemon.statusPollPeriodSeconds must be at least 1", + }, + { + name: "zero controller period", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"controller":{"tagResolutionPeriodSeconds":0}}}]}`, + wantError: "BootcOperatorConfig/custom-config spec.controller.tagResolutionPeriodSeconds must be at least 1", + }, + { + name: "negative daemon period", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"daemon":{"statusPollPeriodSeconds":-1}}}]}`, + wantError: "spec.daemon.statusPollPeriodSeconds must be at least 1", + }, + { + name: "namespace containing comma", body: `{"items":[{"metadata":{"name":"custom-config"},` + + `"spec":{"controller":{"pullSecretNamespaces":["team-a,team-b"]}}}]}`, + wantError: "BootcOperatorConfig/custom-config spec.controller.pullSecretNamespaces[0]: invalid namespace", + }, + {name: "unnamed object", body: `{"items":[{"spec":{}}]}`, wantError: "object without a name"}, + } { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + var resourceRequests []string + wrapped := false + roundTrip := roundTripFunc(func(req *http.Request) (*http.Response, error) { + g.Expect(req.Method).To(Equal(http.MethodGet)) + g.Expect(req.URL.Path).To(Equal(configPath), "list the resource without discovery") + g.Expect(req.URL.Query().Has("timeout")).To(BeTrue()) + g.Expect(req.URL.Query()).To(HaveLen(1), "list without a name or field selector") + resourceRequests = append(resourceRequests, req.URL.Path) + if tt.status != nil { + tt.status.ErrStatus.TypeMeta = metav1.TypeMeta{Kind: "Status", APIVersion: "v1"} + body, err := json.Marshal(tt.status.ErrStatus) + g.Expect(err).NotTo(HaveOccurred()) + return jsonResponse(int(tt.status.ErrStatus.Code), string(body)), nil + } + body := tt.body + if body == "" { + body = `{"items":[{"metadata":{"name":"custom-config","uid":"original",` + + `"generation":3},"spec":{"daemon":{"statusPollPeriodSeconds":17}}}]}` + } + code := tt.code + if code == 0 { + code = http.StatusOK + } + return jsonResponse(code, body), nil + }) + transport := &closeTrackingTransport{RoundTripper: roundTrip} + restConfig := &rest.Config{ + Host: "https://api.example.test", + BearerToken: "test-token", + UserAgent: "config-test", + Transport: transport, + Timeout: time.Minute, + } + restConfig.Wrap( + func(rt http.RoundTripper) http.RoundTripper { wrapped = true; return rt }, + ) + cfg, err := Load(context.Background(), restConfig, configScheme(t)) + g.Expect(transport.closed).To(BeTrue(), "close idle bootstrap connections") + g.Expect(restConfig.Timeout).To(Equal(time.Minute)) + g.Expect(wrapped).To(BeTrue(), "preserve existing transport wrappers") + g.Expect(resourceRequests).To(Equal([]string{configPath})) + if tt.wantError != "" { + g.Expect(err).To(MatchError(ContainSubstring(tt.wantError))) + g.Expect(cfg).To(BeNil()) + return + } + g.Expect(err).NotTo(HaveOccurred()) + if tt.wantNil { + g.Expect(cfg).To(BeNil()) + return + } + g.Expect(cfg.Name).To(Equal("custom-config")) + g.Expect(cfg.UID).To(BeEquivalentTo("original")) + g.Expect(cfg.Generation).To(Equal(int64(3))) + g.Expect(*cfg.Spec.Daemon.StatusPollPeriodSeconds).To(Equal(int32(17))) + }) + } +} + +func TestValidatePullSecretNamespaces(t *testing.T) { + for _, tt := range []struct { + name string + namespaces []string + wantError string + }{ + {name: "omitted"}, + {name: "empty", namespaces: []string{}}, + {name: "valid", namespaces: []string{"team-a", "team-b", strings.Repeat("a", 63)}}, + {name: "empty item", namespaces: []string{""}, wantError: "invalid namespace"}, + {name: "uppercase", namespaces: []string{"Team-A"}, wantError: "invalid namespace"}, + {name: "whitespace", namespaces: []string{" team-a"}, wantError: "invalid namespace"}, + {name: "comma", namespaces: []string{"team-a,team-b"}, wantError: "invalid namespace"}, + {name: "too long", namespaces: []string{strings.Repeat("a", 64)}, wantError: "invalid namespace"}, + {name: "duplicate", namespaces: []string{"team-a", "team-a"}, wantError: "duplicate namespace"}, + } { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + cfg := &bootcv1alpha1.BootcOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "custom-config"}, + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + PullSecretNamespaces: tt.namespaces, + }, + }, + } + err := validate(cfg) + if tt.wantError == "" { + g.Expect(err).NotTo(HaveOccurred()) + } else { + g.Expect(err).To(MatchError(And( + ContainSubstring( + "BootcOperatorConfig/custom-config spec.controller.pullSecretNamespaces[", + ), + ContainSubstring(tt.wantError), + ))) + } + }) + } +} + +func TestLoadRequestFailures(t *testing.T) { + for _, tt := range []struct{ failure, wantError string }{ + {"timeout", "deadline exceeded"}, + {"cancel", "context canceled"}, + {"connection", "connection refused"}, + } { + t.Run(tt.failure, func(t *testing.T) { + g := NewWithT(t) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + injected := false + roundTrip := roundTripFunc(func(req *http.Request) (*http.Response, error) { + g.Expect(req.Method).To(Equal(http.MethodGet)) + g.Expect(req.URL.Path).To(Equal(configPath)) + injected = true + if tt.failure == "connection" { + return nil, errors.New("connection refused") + } + if tt.failure == "cancel" { + cancel() + } + <-req.Context().Done() + return nil, req.Context().Err() + }) + transport := &closeTrackingTransport{RoundTripper: roundTrip} + cfg, err := load( + ctx, + &rest.Config{ + Host: "https://api.example.test", + Transport: transport, + BearerToken: "test-token", + }, + configScheme(t), + 20*time.Millisecond, + ) + g.Expect(transport.closed).To(BeTrue(), "close idle bootstrap connections") + g.Expect(injected).To(BeTrue()) + g.Expect(cfg).To(BeNil()) + g.Expect(err).To(MatchError(ContainSubstring(tt.wantError))) + }) + } +} + +func configScheme(t *testing.T) *runtime.Scheme { + t.Helper() + scheme := runtime.NewScheme() + NewWithT(t).Expect(bootcv1alpha1.AddToScheme(scheme)).To(Succeed()) + return scheme +} + +func jsonResponse(code int, body string) *http.Response { + return &http.Response{ + StatusCode: code, + Header: http.Header{"Content-Type": {"application/json"}}, + Body: io.NopCloser(strings.NewReader(body)), + } +} + +type roundTripFunc func(*http.Request) (*http.Response, error) + +func (fn roundTripFunc) RoundTrip(req *http.Request) (*http.Response, error) { return fn(req) } + +type closeTrackingTransport struct { + http.RoundTripper + closed bool +} + +func (t *closeTrackingTransport) CloseIdleConnections() { t.closed = true } diff --git a/internal/controller/bootcnodepool_controller.go b/internal/controller/bootcnodepool_controller.go index c7a9f9bb..3a0ff86a 100644 --- a/internal/controller/bootcnodepool_controller.go +++ b/internal/controller/bootcnodepool_controller.go @@ -87,6 +87,7 @@ type BootcNodePoolReconciler struct { } // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodepools,verbs=get;list;watch;create;update;patch;delete +// +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcoperatorconfigs,verbs=list // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodepools/status,verbs=get;update;patch // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodepools/finalizers,verbs=update // +kubebuilder:rbac:groups=node.bootc.dev,resources=bootcnodes,verbs=get;list;watch;create;update;patch;delete diff --git a/internal/controller/operatorconfig_test.go b/internal/controller/operatorconfig_test.go new file mode 100644 index 00000000..8432cc83 --- /dev/null +++ b/internal/controller/operatorconfig_test.go @@ -0,0 +1,277 @@ +// SPDX-License-Identifier: Apache-2.0 + +package controller + +import ( + "context" + "encoding/json" + "flag" + "os" + "strings" + "testing" + "time" + + . "github.com/onsi/gomega" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/utils/ptr" + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/envtest" + sigsyaml "sigs.k8s.io/yaml" + + bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" + operatorconfig "github.com/bootc-dev/bootc-operator/internal/config" +) + +// Omitted fields must remain absent so CLI values can supply the fallback. +func TestBootcOperatorConfigDefaults(t *testing.T) { + for _, tc := range []struct { + name, spec string + wantInsecure *bool + wantTag, wantPoll *int32 + }{ + {"empty spec", `{}`, nil, nil, nil}, + {"partial controller", `{"controller":{"allowInsecureRegistry":true}}`, ptr.To(true), nil, nil}, + {"partial daemon", `{"daemon":{"statusPollPeriodSeconds":17}}`, nil, nil, ptr.To(int32(17))}, + {"explicit false", `{"controller":{"allowInsecureRegistry":false,"tagResolutionPeriodSeconds":10}}`, ptr.To(false), ptr.To(int32(10)), nil}, + {"null sections", `{"controller":null,"daemon":null}`, nil, nil, nil}, + {"null fields", `{"controller":{"allowInsecureRegistry":null,"tagResolutionPeriodSeconds":null},"daemon":{"statusPollPeriodSeconds":null}}`, nil, nil, nil}, + {"minimum periods", `{"controller":{"tagResolutionPeriodSeconds":1},"daemon":{"statusPollPeriodSeconds":1}}`, nil, ptr.To(int32(1)), ptr.To(int32(1))}, + {"maximum periods", `{"controller":{"tagResolutionPeriodSeconds":2147483647},"daemon":{"statusPollPeriodSeconds":2147483647}}`, nil, ptr.To(int32(2147483647)), ptr.To(int32(2147483647))}, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + object := operatorConfigObject(t, "cluster", tc.spec) + g.Expect(k8sClient.Create(ctx, object)).To(Succeed()) + t.Cleanup( + func() { g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, object))).To(Succeed()) }, + ) + var got bootcv1alpha1.BootcOperatorConfig + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(object), &got)).To(Succeed()) + g.Expect(got.Spec).To(Equal(bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: tc.wantInsecure, + TagResolutionPeriodSeconds: tc.wantTag, + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: tc.wantPoll, + }, + })) + }) + } +} + +func TestBootcOperatorConfigReleaseExample(t *testing.T) { + g := NewWithT(t) + data, err := os.ReadFile("../../config/samples/bootc_v1alpha1_bootcoperatorconfig.yaml") + g.Expect(err).NotTo(HaveOccurred()) + + var example bootcv1alpha1.BootcOperatorConfig + g.Expect(sigsyaml.UnmarshalStrict(data, &example)).To(Succeed()) + g.Expect(example.APIVersion).To(Equal(bootcv1alpha1.GroupVersion.String())) + g.Expect(example.Kind).To(Equal("BootcOperatorConfig")) + g.Expect(example.Name).To(Equal("cluster")) + g.Expect(example.Spec).To(Equal(bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(false), + TagResolutionPeriodSeconds: ptr.To(bootcv1alpha1.DefaultTagResolutionPeriodSeconds), + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(bootcv1alpha1.DefaultStatusPollPeriodSeconds), + }, + })) + g.Expect(k8sClient.Create(context.Background(), &example, + client.DryRunAll, client.FieldValidation(metav1.FieldValidationStrict))).To(Succeed()) +} + +func TestBootcOperatorConfigValidation(t *testing.T) { + for _, tc := range []struct { + name string + spec string + field string + badRequest bool + }{ + {"missing spec", "", "spec", false}, + {"null spec", `null`, "spec", false}, + {"zero tag period", `{"controller":{"tagResolutionPeriodSeconds":0}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"negative tag period", `{"controller":{"tagResolutionPeriodSeconds":-1}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"zero poll period", `{"daemon":{"statusPollPeriodSeconds":0}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"negative poll period", `{"daemon":{"statusPollPeriodSeconds":-1}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"overflowing tag period", `{"controller":{"tagResolutionPeriodSeconds":2147483648}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"overflowing poll period", `{"daemon":{"statusPollPeriodSeconds":2147483648}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"string period", `{"daemon":{"statusPollPeriodSeconds":"10s"}}`, "spec.daemon.statusPollPeriodSeconds", false}, + {"fractional period", `{"controller":{"tagResolutionPeriodSeconds":1.5}}`, "spec.controller.tagResolutionPeriodSeconds", false}, + {"string boolean", `{"controller":{"allowInsecureRegistry":"true"}}`, "spec.controller.allowInsecureRegistry", false}, + {"namespace list type", `{"controller":{"pullSecretNamespaces":"team-a"}}`, "spec.controller.pullSecretNamespaces", false}, + {"namespace item type", `{"controller":{"pullSecretNamespaces":[1]}}`, "spec.controller.pullSecretNamespaces", false}, + {"empty namespace", `{"controller":{"pullSecretNamespaces":[""]}}`, "spec.controller.pullSecretNamespaces", false}, + {"uppercase namespace", `{"controller":{"pullSecretNamespaces":["Team-A"]}}`, "spec.controller.pullSecretNamespaces", false}, + {"namespace whitespace", `{"controller":{"pullSecretNamespaces":[" team-a"]}}`, "spec.controller.pullSecretNamespaces", false}, + {"namespace comma", `{"controller":{"pullSecretNamespaces":["team-a,team-b"]}}`, "spec.controller.pullSecretNamespaces", false}, + {"namespace too long", `{"controller":{"pullSecretNamespaces":["` + strings.Repeat("a", 64) + `"]}}`, "spec.controller.pullSecretNamespaces", false}, + {"duplicate namespaces", `{"controller":{"pullSecretNamespaces":["team-a","team-a"]}}`, "spec.controller.pullSecretNamespaces", false}, + {"unknown field", `{"controller":{"tagResolutionPeriodSecond":10}}`, "spec.controller.tagResolutionPeriodSecond", true}, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + object := operatorConfigObject(t, "cluster", tc.spec) + err := k8sClient.Create(context.Background(), object, + client.DryRunAll, client.FieldValidation(metav1.FieldValidationStrict)) + if tc.badRequest { + g.Expect(err).To(MatchError(apierrors.IsBadRequest, "IsBadRequest")) + } else { + g.Expect(err).To(MatchError(apierrors.IsInvalid, "IsInvalid")) + } + g.Expect(err.Error()).To(ContainSubstring(tc.field)) + }) + } +} + +func TestBootcOperatorConfigRoundTrip(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + loaded, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(loaded).To(BeNil(), "an absent instance permits startup defaults") + config := &bootcv1alpha1.BootcOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "custom-config"}, + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: ptr.To(true), + TagResolutionPeriodSeconds: ptr.To(int32(10)), + PullSecretNamespaces: []string{"team-a", strings.Repeat("a", 63)}, + }, + Daemon: &bootcv1alpha1.OperatorDaemonConfig{ + StatusPollPeriodSeconds: ptr.To(int32(17)), + }, + }, + } + wantSpec := *config.Spec.DeepCopy() + g.Expect(k8sClient.Create(ctx, config)).To(Succeed()) + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, config))).To(Succeed()) + }) + got, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Name).To(Equal("custom-config")) + g.Expect(got.Spec).To(Equal(wantSpec)) + + startup := got + updated := got.DeepCopy() + updated.Spec.Controller.AllowInsecureRegistry = ptr.To(false) + updated.Spec.Controller.PullSecretNamespaces = []string{"team-b"} + updated.Spec.Daemon.StatusPollPeriodSeconds = ptr.To(int32(23)) + wantSpec = *updated.Spec.DeepCopy() + g.Expect(k8sClient.Update(ctx, updated)).To(Succeed()) + got, err = operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).NotTo(BeNil()) + g.Expect(got.Spec).To(Equal(wantSpec)) + g.Expect(got.UID).To(Equal(config.UID)) + g.Expect(got.Generation).To(Equal(config.Generation + 1)) + g.Expect(startup.Spec). + To(Equal(config.Spec), "previously loaded configuration remains a snapshot") + + g.Expect(k8sClient.Delete(ctx, got)).To(Succeed()) + loaded, err = operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(loaded).To(BeNil(), "deleting the instance restores defaults on the next load") +} + +// Typed writes must preserve an explicitly empty list, which overrides CLI +// namespaces. Explicit values matching built-in defaults also override CLI values. +func TestBootcOperatorConfigEmptyNamespaces(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + cfg := &bootcv1alpha1.BootcOperatorConfig{ + ObjectMeta: metav1.ObjectMeta{Name: "cluster"}, + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + PullSecretNamespaces: []string{}, + TagResolutionPeriodSeconds: ptr.To(int32(300)), + }, + }, + } + g.Expect(k8sClient.Create(ctx, cfg)).To(Succeed()) + t.Cleanup(func() { g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, cfg))).To(Succeed()) }) + for _, update := range []bool{false, true} { + if update { + cfg.Labels = map[string]string{"test": "updated"} + g.Expect(k8sClient.Update(ctx, cfg)).To(Succeed()) + } + g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(cfg), cfg)).To(Succeed()) + g.Expect(cfg.Spec.Controller.PullSecretNamespaces).To(Equal([]string{})) + loaded, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + fs := flag.NewFlagSet(t.Name(), flag.ContinueOnError) + namespaces := fs.String(operatorconfig.PullSecretNamespacesFlag, "", "") + interval := fs.Duration(operatorconfig.TagResolutionIntervalFlag, 5*time.Minute, "") + g.Expect(fs.Parse([]string{"--pull-secret-namespaces=team-a", "--tag-resolution-interval=10s"})). + To(Succeed()) + g.Expect(operatorconfig.ApplyToFlags(fs, loaded)).To(Succeed()) + g.Expect(*namespaces).To(BeEmpty()) + g.Expect(*interval). + To(Equal(5*time.Minute), "explicit default-valued fields override CLI") + } + cfg.Spec.Controller.PullSecretNamespaces = nil + cfg.Spec.Controller.TagResolutionPeriodSeconds = nil + g.Expect(k8sClient.Update(ctx, cfg)).To(Succeed()) + loaded, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(loaded.Spec.Controller.PullSecretNamespaces).To(BeNil()) + g.Expect(loaded.Spec.Controller.TagResolutionPeriodSeconds).To(BeNil()) +} + +func TestBootcOperatorConfigMultipleInstances(t *testing.T) { + g := NewWithT(t) + ctx := context.Background() + for _, name := range []string{"operator-one", "operator-two"} { + object := operatorConfigObject(t, name, `{}`) + g.Expect(k8sClient.Create(ctx, object)).To(Succeed(), "the API allows arbitrary names") + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(k8sClient.Delete(ctx, object))).To(Succeed()) + }) + } + + loaded, err := operatorconfig.Load(ctx, testEnv.Config, k8sClient.Scheme()) + g.Expect(loaded).To(BeNil()) + g.Expect(err).To(HaveOccurred()) + g.Expect(err.Error()).To(And( + ContainSubstring("operator-one"), + ContainSubstring("operator-two"), + )) +} + +func TestBootcOperatorConfigMissingCRD(t *testing.T) { + g := NewWithT(t) + // A separate API server keeps the missing-CRD case isolated from the suite. + env := &envtest.Environment{} + restConfig, err := env.Start() + g.Expect(err).NotTo(HaveOccurred()) + t.Cleanup(func() { g.Expect(env.Stop()).To(Succeed()) }) + + loaded, err := operatorconfig.Load(context.Background(), restConfig, k8sClient.Scheme()) + g.Expect(err).To(MatchError(apierrors.IsNotFound, "IsNotFound")) + g.Expect(err.Error()).To(ContainSubstring("ensure the bootcoperatorconfigs CRD is installed")) + g.Expect(loaded).To(BeNil()) +} + +// Use unstructured objects to exercise missing, null, and invalid fields that +// typed Go objects cannot represent on the wire. +func operatorConfigObject(t *testing.T, name, spec string) *unstructured.Unstructured { + t.Helper() + object := &unstructured.Unstructured{Object: map[string]any{ + "apiVersion": bootcv1alpha1.GroupVersion.String(), + "kind": "BootcOperatorConfig", + "metadata": map[string]any{"name": name}, + }} + if spec != "" { + var value any + NewWithT(t).Expect(json.Unmarshal([]byte(spec), &value)).To(Succeed()) + object.Object["spec"] = value + } + return object +} diff --git a/test/e2e/pullsecret_namespace_test.go b/test/e2e/pullsecret_namespace_test.go index 2c9c8b87..04d655af 100644 --- a/test/e2e/pullsecret_namespace_test.go +++ b/test/e2e/pullsecret_namespace_test.go @@ -12,7 +12,9 @@ import ( "time" . "github.com/onsi/gomega" + appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + rbacv1 "k8s.io/api/rbac/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" @@ -110,6 +112,135 @@ func TestPullSecretNamespaces(t *testing.T) { t.Logf("Node %q updated with a pull secret from namespace %q", nodeName, allowedNS) } +// TestOperatorConfigPullSecretNamespaces changes shared startup configuration, +// so it must stay serial. No node is provisioned: reconciling a digest-based pool +// proves that the controller can read a Secret through its namespace-scoped cache. +func TestOperatorConfigPullSecretNamespaces(t *testing.T) { + e2eutil.Providers(t, "bink") + g := NewWithT(t) + g.SetDefaultEventuallyTimeout(pollTimeout) + g.SetDefaultEventuallyPollingInterval(pollInterval) + env := e2eutil.New(t) + ctx := context.Background() + var originalDeploy appsv1.Deployment + g.Expect(env.Client.Get(ctx, operatorDeployKey(), &originalDeploy)).To(Succeed()) + originalConfig, err := readOperatorConfig(ctx, env.Client) + g.Expect(err).NotTo(HaveOccurred()) + + namespaces := []string{env.TestID() + "-allowed", env.TestID() + "-denied"} + for _, name := range namespaces { + ns := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: name}} + g.Expect(env.Client.Create(ctx, ns)).To(Succeed()) + t.Cleanup( + func() { g.Expect(client.IgnoreNotFound(env.Client.Delete(ctx, ns))).To(Succeed()) }, + ) + secret := &corev1.Secret{ + ObjectMeta: metav1.ObjectMeta{Name: "pull-secret", Namespace: name}, + Type: corev1.SecretTypeDockerConfigJson, + Data: map[string][]byte{corev1.DockerConfigJsonKey: []byte(`{"auths":{}}`)}, + } + g.Expect(env.Client.Create(ctx, secret)).To(Succeed()) + } + role := &rbacv1.Role{ + ObjectMeta: metav1.ObjectMeta{Name: "pull-secrets", Namespace: namespaces[0]}, + Rules: []rbacv1.PolicyRule{ + { + APIGroups: []string{""}, + Resources: []string{"secrets"}, + Verbs: []string{"get", "list", "watch"}, + }, + }, + } + g.Expect(env.Client.Create(ctx, role)).To(Succeed()) + binding := &rbacv1.RoleBinding{ + ObjectMeta: metav1.ObjectMeta{Name: role.Name, Namespace: role.Namespace}, + RoleRef: rbacv1.RoleRef{APIGroup: rbacv1.GroupName, Kind: "Role", Name: role.Name}, + Subjects: []rbacv1.Subject{ + { + Kind: "ServiceAccount", + Name: originalDeploy.Spec.Template.Spec.ServiceAccountName, + Namespace: originalDeploy.Namespace, + }, + }, + } + g.Expect(env.Client.Create(ctx, binding)).To(Succeed()) + + var originalArgs []string + for _, cont := range originalDeploy.Spec.Template.Spec.Containers { + if cont.Name == "manager" { + originalArgs = cont.Args + } + } + t.Cleanup(func() { + g.Expect(restoreOperatorConfig(ctx, env.Client, originalConfig)).To(Succeed()) + setControllerArgs(t, g, ctx, env.Client, originalArgs) + restartOperator(t) + waitForOperatorReady(t, g, ctx, env.Client) + }) + cfg := originalConfig.DeepCopy() + if cfg == nil { + cfg = &bootcv1alpha1.BootcOperatorConfig{ObjectMeta: metav1.ObjectMeta{Name: env.TestID()}} + } + if cfg.Spec.Controller == nil { + cfg.Spec.Controller = &bootcv1alpha1.OperatorControllerConfig{} + } + cfg.Spec.Controller.PullSecretNamespaces = []string{namespaces[0]} + g.Expect(restoreOperatorConfig(ctx, env.Client, cfg)).To(Succeed()) + args, _ := splitNamespaceArgs(originalArgs) + setControllerArgs(t, g, ctx, env.Client, args) + restartOperator(t) + waitForOperatorReady(t, g, ctx, env.Client) + + pool := env.NewPool("config-secret", env.NodeImageDigestedPullSpec(), + testutil.WithPullSecret("pull-secret", namespaces[1])) + g.Expect(env.Client.Create(ctx, pool)).To(Succeed()) + t.Cleanup(func() { + g.Expect(client.IgnoreNotFound(env.Client.Delete(ctx, pool))).To(Succeed()) + g.Eventually(func() error { return env.Client.Get(ctx, client.ObjectKeyFromObject(pool), pool) }). + Should(MatchError(apierrors.IsNotFound, "IsNotFound")) + }) + conditions := func() ([]metav1.Condition, error) { + var got bootcv1alpha1.BootcNodePool + err := env.Client.Get(ctx, client.ObjectKeyFromObject(pool), &got) + return got.Status.Conditions, err + } + g.Eventually(conditions).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionTrue), + HaveField("Reason", bootcv1alpha1.PoolSecretError), + HaveField("Message", ContainSubstring(namespaces[1])), + ))) + modified := pool.DeepCopy() + modified.Spec.PullSecretRef.Namespace = namespaces[0] + g.Expect(env.Client.Patch(ctx, modified, client.MergeFrom(pool))).To(Succeed()) + g.Eventually(conditions).Should(ContainElement(And( + HaveField("Type", bootcv1alpha1.PoolDegraded), + HaveField("Status", metav1.ConditionFalse), + HaveField("Reason", bootcv1alpha1.PoolHealthy), + ))) +} + +func setControllerArgs( + t *testing.T, + g Gomega, + ctx context.Context, + c client.Client, + args []string, +) { + t.Helper() + var d appsv1.Deployment + g.Expect(c.Get(ctx, operatorDeployKey(), &d)).To(Succeed()) + original := d.DeepCopy() + for i := range d.Spec.Template.Spec.Containers { + if d.Spec.Template.Spec.Containers[i].Name == "manager" { + d.Spec.Template.Spec.Containers[i].Args = args + g.Expect(c.Patch(ctx, &d, client.MergeFrom(original))).To(Succeed()) + return + } + } + t.Fatal("manager container not found") +} + // registrySecret returns a dockerconfigjson Secret with the credentials of // the authenticated e2e registry. func registrySecret(env *e2eutil.Env, name, namespace string) *corev1.Secret { diff --git a/test/e2e/upgrade_test.go b/test/e2e/upgrade_test.go index 7c9ec0b9..c30bd20a 100644 --- a/test/e2e/upgrade_test.go +++ b/test/e2e/upgrade_test.go @@ -5,6 +5,8 @@ package e2e import ( "context" "encoding/json" + "flag" + "fmt" "os" "os/exec" "path/filepath" @@ -17,9 +19,12 @@ import ( corev1 "k8s.io/api/core/v1" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "sigs.k8s.io/controller-runtime/pkg/client" bootcv1alpha1 "github.com/bootc-dev/bootc-operator/api/v1alpha1" + "github.com/bootc-dev/bootc-operator/internal/config" + "github.com/bootc-dev/bootc-operator/internal/controller" "github.com/bootc-dev/bootc-operator/test/e2e/e2eutil" testutil "github.com/bootc-dev/bootc-operator/test/util" ) @@ -29,6 +34,7 @@ const helmReleaseName = "bootc-operator" var operatorCRDNames = []string{ "bootcnodepools.node.bootc.dev", "bootcnodes.node.bootc.dev", + "bootcoperatorconfigs.node.bootc.dev", } // TestOperatorUpgrade installs the released version of the operator @@ -67,30 +73,37 @@ func TestOperatorUpgrade(t *testing.T) { g.Expect(env.Client.Get(ctx, operatorDeployKey(), &origDeploy)).To(Succeed()) currentImg := containerImage(t, origDeploy.Spec.Template.Spec.Containers, "manager") extraArgs := controllerExtraArgs(t, origDeploy.Spec.Template.Spec.Containers) - pullSecretNamespaces := controllerPullSecretNamespaces(t, g, ctx, env.Client) + helmValues := readHelmControllerValues(t) + pullSecretNamespaces := helmValues.PullSecretNamespaces + // Preserve explicit namespace overrides separately from Helm-managed grants. + _, namespaceOverrides := splitNamespaceArgs(helmValues.Controller.ExtraArgs) + extraArgs = append(extraArgs, namespaceOverrides...) t.Logf("Current operator image: %s", currentImg) t.Logf("Released operator tag: %s, image: %s, chart: %s", releaseTag, releasedImg, releasedChart) - // Phase 1: Uninstall the current operator and re-install the - // released version from its published chart, the way users - // upgrading from it have it deployed. The current chart may not work - // with the released image. - uninstallOperator(t, g, ctx, env) - - // Restore the current operator even if the released one fails to - // install, so the tests that run after this one have an operator. + originalConfig, err := readOperatorConfig(ctx, env.Client) + g.Expect(err).NotTo(HaveOccurred()) + // Uninstall removes CRDs and their instances. Register recovery first. t.Cleanup(func() { t.Logf("Restoring operator to current version...") helmUpgradeOperator(t, chartPath, currentImg, extraArgs, pullSecretNamespaces) + waitForOperatorConfigCRD(t, g, ctx, env.Client) + g.Expect(restoreOperatorConfig(ctx, env.Client, originalConfig)).To(Succeed(), + "restore administrator-owned operator configuration") + restartOperator(t) waitForOperatorReady(t, g, ctx, env.Client) t.Logf("Operator restored") }) + // Phase 1: Replace the current operator with its published release chart. + uninstallOperator(t, g, ctx, env) + // The released image predates BootcOperatorConfig and needs bink's + // registry settings through its command-line flags. + releasedArgs := releasedControllerArgs(originalConfig, extraArgs) helmInstallOperator(t, releasedChart, releasedChartVersion, - releasedImg, extraArgs) + releasedImg, releasedArgs) waitForOperatorReady(t, g, ctx, env.Client) - t.Logf("Released operator %s installed via Helm", releaseTag) // Phase 2: Verify the released version works. @@ -103,8 +116,17 @@ func TestOperatorUpgrade(t *testing.T) { t.Logf("Node %q is Idle with released operator", nodeName) - // Phase 3: Upgrade via helm upgrade to the current version. + // Phase 3: Upgrade via Helm to the current version. helmUpgradeOperator(t, chartPath, currentImg, extraArgs, pullSecretNamespaces) + waitForOperatorConfigCRD(t, g, ctx, env.Client) + config, err := readOperatorConfig(ctx, env.Client) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(config).To(BeNil(), "installation must not create a configuration instance") + g.Expect(restoreOperatorConfig(ctx, env.Client, originalConfig)).To(Succeed(), + "restore administrator-owned operator configuration before upgrade checks") + if originalConfig != nil { + restartOperator(t) + } waitForOperatorReady(t, g, ctx, env.Client) t.Logf("Upgraded operator to current version via Helm") @@ -140,8 +162,7 @@ func TestOperatorUpgrade(t *testing.T) { // uninstallOperator removes the operator. Custom resources are deleted // first while the operator is still running so it can handle finalizer -// removal. CRDs are bundled as a subchart dependency so they are -// removed together with the main release. +// removal. The test also removes a standalone CRD release if present. func uninstallOperator( t *testing.T, g Gomega, @@ -212,7 +233,8 @@ func helmInstallOperator( } // helmUpgradeOperator upgrades the operator to the local chart via helm -// upgrade. The release namespace is always allowed, so including it in +// upgrade, installing it if cleanup follows a failed release installation. +// The release namespace is always allowed, so including it in // pullSecretNamespaces is harmless. func helmUpgradeOperator( t *testing.T, @@ -313,6 +335,68 @@ func splitImage(ref string) (repo, tag string) { return ref, "latest" } +// releasedControllerArgs carries CR settings to releases that only support CLI +// flags. Present CR fields come last and override Deployment arguments. +func releasedControllerArgs(cfg *bootcv1alpha1.BootcOperatorConfig, explicit []string) []string { + args, _ := splitNamespaceArgs(explicit) + if cfg != nil && cfg.Spec.Controller != nil { + c := cfg.Spec.Controller + if c.AllowInsecureRegistry != nil { + args = append( + args, + fmt.Sprintf("--%s=%t", config.AllowInsecureRegistryFlag, *c.AllowInsecureRegistry), + ) + } + if c.TagResolutionPeriodSeconds != nil { + period := time.Duration(*c.TagResolutionPeriodSeconds) * time.Second + args = append(args, "--"+config.TagResolutionIntervalFlag+"="+period.String()) + } + } + return args +} + +// TestReleasedControllerArgs verifies that older releases receive supported CR +// settings as flags with CR-first precedence, without unsupported namespace flags. +func TestReleasedControllerArgs(t *testing.T) { + allowInsecure, seconds := true, int32(17) + cfg := &bootcv1alpha1.BootcOperatorConfig{ + Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + AllowInsecureRegistry: &allowInsecure, TagResolutionPeriodSeconds: &seconds, + PullSecretNamespaces: []string{"from-cr"}, + }, + }, + } + for _, tt := range []struct { + name string + cfg *bootcv1alpha1.BootcOperatorConfig + explicit []string + wantHTTP bool + wantWait time.Duration + }{ + {name: "absent config", wantWait: 5 * time.Minute}, + {name: "omitted fields", cfg: &bootcv1alpha1.BootcOperatorConfig{}, wantWait: 5 * time.Minute}, + {name: "saved settings", cfg: cfg, wantHTTP: true, wantWait: 17 * time.Second}, + {name: "omitted field retains CLI precision", cfg: &bootcv1alpha1.BootcOperatorConfig{}, explicit: []string{"--tag-resolution-interval=1500ms"}, wantWait: 1500 * time.Millisecond}, + {name: "omit unsupported namespace flag", cfg: cfg, explicit: []string{"--pull-secret-namespaces", "team-a"}, wantHTTP: true, wantWait: 17 * time.Second}, + { + name: "CR fields override arguments", cfg: cfg, + explicit: []string{"--allow-insecure-registry=false", "--tag-resolution-interval=1500ms"}, + wantHTTP: true, wantWait: 17 * time.Second, + }, + } { + t.Run(tt.name, func(t *testing.T) { + g := NewWithT(t) + fs := flag.NewFlagSet(tt.name, flag.ContinueOnError) + http := fs.Bool(config.AllowInsecureRegistryFlag, false, "") + wait := fs.Duration(config.TagResolutionIntervalFlag, 5*time.Minute, "") + g.Expect(fs.Parse(releasedControllerArgs(tt.cfg, tt.explicit))).To(Succeed()) + g.Expect(*http).To(Equal(tt.wantHTTP)) + g.Expect(*wait).To(Equal(tt.wantWait)) + }) + } +} + // controllerExtraArgs returns the controller's extra arguments, // filtering out the flags that the Helm chart always includes. func controllerExtraArgs(t *testing.T, containers []corev1.Container) []string { @@ -322,11 +406,11 @@ func controllerExtraArgs(t *testing.T, containers []corev1.Container) []string { continue } var extra []string - for _, arg := range c.Args { + args, _ := splitNamespaceArgs(c.Args) + for _, arg := range args { switch { case arg == "--leader-elect", - strings.HasPrefix(arg, "--health-probe-bind-address="), - strings.HasPrefix(arg, "--pull-secret-namespaces="): + strings.HasPrefix(arg, "--health-probe-bind-address="): default: extra = append(extra, arg) } @@ -337,29 +421,127 @@ func controllerExtraArgs(t *testing.T, containers []corev1.Container) []string { return nil } -// controllerPullSecretNamespaces returns the namespaces the running -// controller allows pull secrets from, as given by its -// --pull-secret-namespaces flag. +// controllerPullSecretNamespaces resolves the startup settings from CR and CLI. +// Call it only when the Deployment has restarted after configuration changes. func controllerPullSecretNamespaces( - t *testing.T, - g Gomega, - ctx context.Context, - c client.Client, + t *testing.T, g Gomega, ctx context.Context, c client.Client, ) []string { t.Helper() var d appsv1.Deployment g.Expect(c.Get(ctx, operatorDeployKey(), &d)).To(Succeed()) + cfg, err := readOperatorConfig(ctx, c) + g.Expect(err).NotTo(HaveOccurred()) for _, cont := range d.Spec.Template.Spec.Containers { - if cont.Name != "manager" { - continue + if cont.Name == "manager" { + namespaces, err := effectivePullSecretNamespaces(cont.Args, cfg, d.Namespace) + g.Expect(err).NotTo(HaveOccurred()) + return namespaces } - for _, arg := range cont.Args { - if v, ok := strings.CutPrefix(arg, "--pull-secret-namespaces="); ok { - return strings.Split(v, ",") + } + t.Fatal("manager container not found") + return nil +} + +func effectivePullSecretNamespaces( + args []string, cfg *bootcv1alpha1.BootcOperatorConfig, operatorNamespace string, +) ([]string, error) { + fs := flag.NewFlagSet("controller namespaces", flag.ContinueOnError) + value := fs.String(config.PullSecretNamespacesFlag, "", "") + _, namespaceArgs := splitNamespaceArgs(args) + if err := fs.Parse(namespaceArgs); err != nil { + return nil, err + } + if err := config.ApplyToFlags(fs, cfg); err != nil { + return nil, err + } + return controller.ParsePullSecretNamespaces(*value, operatorNamespace) +} + +// splitNamespaceArgs handles both flag forms and retains their order so the last +// explicit value wins. Older released operators must not receive this new flag. +func splitNamespaceArgs(args []string) (other, namespaces []string) { + for i := 0; i < len(args); i++ { + name := strings.TrimPrefix(args[i], "-") + name = strings.TrimPrefix(name, "-") + if name == config.PullSecretNamespacesFlag { + namespaces = append(namespaces, args[i]) + if i+1 < len(args) { + i++ + namespaces = append(namespaces, args[i]) } + } else if strings.HasPrefix(name, config.PullSecretNamespacesFlag+"=") { + namespaces = append(namespaces, args[i]) + } else { + other = append(other, args[i]) } } - return nil + return other, namespaces +} + +type helmControllerValues struct { + PullSecretNamespaces []string `json:"pullSecretNamespaces"` + Controller struct { + ExtraArgs []string `json:"extraArgs"` + } `json:"controller"` +} + +// Read actual release values, rather than turning CR-derived namespaces into +// Helm-managed RBAC or a persistent CLI fallback during recovery. +func readHelmControllerValues(t *testing.T) helmControllerValues { + t.Helper() + args := []string{ + "get", + "values", + helmReleaseName, + "--namespace", + testutil.OperatorNamespaceName, + "--output", + "json", + } + if kc := os.Getenv("KUBECONFIG"); kc != "" { + args = append([]string{"--kubeconfig", kc}, args...) + } + out, err := exec.Command("helm", args...).Output() + if err != nil { + t.Fatalf("read original Helm controller settings: %v", err) + } + var values helmControllerValues + if err := json.Unmarshal(out, &values); err != nil { + t.Fatalf("decode original Helm controller settings: %v", err) + } + return values +} + +// TestEffectivePullSecretNamespaces checks CR-first namespace selection, CLI +// fallback for omitted fields, and the operator namespace always remaining allowed. +func TestEffectivePullSecretNamespaces(t *testing.T) { + cfg := &bootcv1alpha1.BootcOperatorConfig{Spec: bootcv1alpha1.BootcOperatorConfigSpec{ + Controller: &bootcv1alpha1.OperatorControllerConfig{ + PullSecretNamespaces: []string{"from-cr"}, + }, + }} + empty := cfg.DeepCopy() + empty.Spec.Controller.PullSecretNamespaces = []string{} + for _, tc := range []struct { + name string + cfg *bootcv1alpha1.BootcOperatorConfig + args, want []string + }{ + {"defaults", nil, nil, []string{"operator"}}, + {"CR only", cfg, nil, []string{"operator", "from-cr"}}, + {"empty CR overrides CLI", empty, []string{"--pull-secret-namespaces=from-cli"}, []string{"operator"}}, + {"CLI only", nil, []string{"--pull-secret-namespaces=from-cli"}, []string{"operator", "from-cli"}}, + {"CR overrides CLI", cfg, []string{"--pull-secret-namespaces", "from-cli"}, []string{"operator", "from-cr"}}, + {"CR overrides empty CLI", cfg, []string{"--pull-secret-namespaces="}, []string{"operator", "from-cr"}}, + {"last CLI flag wins without CR field", nil, []string{"--pull-secret-namespaces=first", "--pull-secret-namespaces=last"}, []string{"operator", "last"}}, + } { + t.Run(tc.name, func(t *testing.T) { + g := NewWithT(t) + got, err := effectivePullSecretNamespaces(tc.args, tc.cfg, "operator") + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(got).To(Equal(tc.want)) + }) + } } // waitForOperatorReady polls until the operator deployment has one available and up-to-date replica. @@ -373,6 +555,7 @@ func waitForOperatorReady( g.Eventually(func(g Gomega) { var d appsv1.Deployment g.Expect(c.Get(ctx, operatorDeployKey(), &d)).To(Succeed()) + g.Expect(d.Status.ObservedGeneration).To(BeNumerically(">=", d.Generation)) g.Expect(d.Status.Replicas).To(Equal(int32(1))) g.Expect(d.Status.UpdatedReplicas).To(Equal(int32(1))) g.Expect(d.Status.AvailableReplicas).To(Equal(int32(1))) @@ -380,6 +563,106 @@ func waitForOperatorReady( "expected operator deployment to be ready") } +// readOperatorConfig returns the optional administrator-owned instance. An +// ambiguous configuration should fail the upgrade instead of being discarded. +func readOperatorConfig( + ctx context.Context, + c client.Client, +) (*bootcv1alpha1.BootcOperatorConfig, error) { + var configs bootcv1alpha1.BootcOperatorConfigList + if err := c.List(ctx, &configs); err != nil { + return nil, fmt.Errorf("list operator configurations: %w", err) + } + switch len(configs.Items) { + case 0: + return nil, nil + case 1: + return configs.Items[0].DeepCopy(), nil + default: + return nil, fmt.Errorf( + "expected at most one operator configuration, found %d", + len(configs.Items), + ) + } +} + +// restoreOperatorConfig restores only administrator-owned fields after CRD +// deletion. A different instance name is an error rather than an implicit rename. +func restoreOperatorConfig( + ctx context.Context, + c client.Client, + original *bootcv1alpha1.BootcOperatorConfig, +) error { + current, err := readOperatorConfig(ctx, c) + if err != nil { + return err + } + if original == nil { + if current == nil { + return nil + } + return c.Delete(ctx, current) + } + if current != nil && current.Name != original.Name { + return fmt.Errorf( + "restore operator configuration %q: unexpected current instance %q", + original.Name, + current.Name, + ) + } + if current == nil { + restored := original.DeepCopy() + restored.ObjectMeta = metav1.ObjectMeta{ + Name: original.Name, + Labels: original.Labels, + Annotations: original.Annotations, + } + return c.Create(ctx, restored) + } + current.Spec = original.Spec + current.Labels = original.Labels + current.Annotations = original.Annotations + return c.Update(ctx, current) +} + +func waitForOperatorConfigCRD(t *testing.T, g Gomega, ctx context.Context, c client.Client) { + t.Helper() + g.Eventually(func() ([]apiextensionsv1.CustomResourceDefinitionCondition, error) { + var crd apiextensionsv1.CustomResourceDefinition + err := c.Get(ctx, client.ObjectKey{Name: "bootcoperatorconfigs.node.bootc.dev"}, &crd) + return crd.Status.Conditions, err + }).WithTimeout(time.Minute).Should(ContainElement(And( + HaveField("Type", apiextensionsv1.Established), + HaveField("Status", apiextensionsv1.ConditionTrue), + )), "waiting for the operator configuration API to be established") +} + +// Both components may have started with defaults before configuration was +// restored. Restart and await each workload so later tests see the saved values. +func restartOperator(t *testing.T) { + t.Helper() + workloads := []string{ + "deployment/" + operatorDeployKey().Name, + "daemonset/" + operatorDaemonSetKey().Name, + } + for _, workload := range workloads { + for _, command := range [][]string{ + {"rollout", "restart", workload}, + {"rollout", "status", workload, "--timeout=3m"}, + } { + ctx, cancel := context.WithTimeout(context.Background(), 4*time.Minute) + args := append([]string{ + "--kubeconfig", os.Getenv("KUBECONFIG"), "-n", testutil.OperatorNamespaceName, + }, command...) + out, err := exec.CommandContext(ctx, "kubectl", args...).CombinedOutput() + cancel() + if err != nil { + t.Fatalf("restart operator workload %s: %v\n%s", workload, err, out) + } + } + } +} + // operatorDeployKey returns the namespaced name of the operator Deployment. func operatorDeployKey() client.ObjectKey { return client.ObjectKey{