From e1b25c8f0b23970de5f30f02fc6e0c2fd5b92a31 Mon Sep 17 00:00:00 2001 From: David Hurta Date: Wed, 1 Apr 2026 14:52:12 +0200 Subject: [PATCH 1/2] Remove cluster role binding referencing default service account To ensure the default SA does not have cluster admin permissions. To achieve this, rendering logic needs to be updated as well. During cluster bootstrap, the installer calls rendering commands of specific components required for the bootstrap [1]. These rendered manifests are then applied by the cluster-bootstrap component [2]. The cluster-bootstrap component applies all the non-bootstrap manifests as they are [3]. At no stage is the delete annotation [4] taken into account, and thus the CRB would keep getting applied during installations and getting removed only during cluster upgrades due to the annotation. This would prohibit us from ever removing the manifest file from the repository, as a freshly installed cluster upgrading to a version where manifest does not exist would result in the CRB being applied till manually removed, causing a security concern. Teach the rendering command to respect the delete annotation to allow us to remove such manifests. Add the delete annotation and move the CRB into a lower run-level where we can safely remove manifests and not take up space in the used levels. [1]: https://github.com/openshift/installer/blob/c93ae9fc74d7fa0f478fa250de2ba702f84a0a21/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template [2]: https://github.com/openshift/installer/blob/c93ae9fc74d7fa0f478fa250de2ba702f84a0a21/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template#L576 [3]: https://github.com/openshift/cluster-bootstrap/blob/b23c6ce3df43aed15158e999239694ec75371f18/pkg/start/start.go#L142 [4]: https://github.com/openshift/enhancements/blob/master/enhancements/update/object-removal-manifest-annotation.md --- cmd/cluster-version-operator/render.go | 8 +- ...er-version-operator_90_roles-default.yaml} | 1 + pkg/payload/render.go | 6 ++ pkg/payload/render_test.go | 75 ++++++++++++++++++- 4 files changed, 84 insertions(+), 6 deletions(-) rename install/{0000_00_cluster-version-operator_03_roles-default.yaml => 0000_00_cluster-version-operator_90_roles-default.yaml} (90%) diff --git a/cmd/cluster-version-operator/render.go b/cmd/cluster-version-operator/render.go index 068c9a1e3f..2502241c3c 100644 --- a/cmd/cluster-version-operator/render.go +++ b/cmd/cluster-version-operator/render.go @@ -14,9 +14,11 @@ import ( var ( renderCmd = &cobra.Command{ Use: "render", - Short: "Renders the UpdatePayload to disk.", - Long: "", - Run: runRenderCmd, + Short: "Renders and filters release payload manifests for cluster bootstrap.", + Long: `Renders and filters CVO manifests and specific release payload manifests. + +Called by the installer as part of cluster bootstrap to generate the initial manifests related to the CVO.`, + Run: runRenderCmd, } renderOpts struct { diff --git a/install/0000_00_cluster-version-operator_03_roles-default.yaml b/install/0000_00_cluster-version-operator_90_roles-default.yaml similarity index 90% rename from install/0000_00_cluster-version-operator_03_roles-default.yaml rename to install/0000_00_cluster-version-operator_90_roles-default.yaml index 34c14b4602..83cfa5fed4 100644 --- a/install/0000_00_cluster-version-operator_03_roles-default.yaml +++ b/install/0000_00_cluster-version-operator_90_roles-default.yaml @@ -4,6 +4,7 @@ metadata: name: cluster-version-operator annotations: include.release.openshift.io/self-managed-high-availability: "true" + release.openshift.io/delete: "true" roleRef: kind: ClusterRole name: cluster-admin diff --git a/pkg/payload/render.go b/pkg/payload/render.go index fc075a6e5c..5fae362ebf 100644 --- a/pkg/payload/render.go +++ b/pkg/payload/render.go @@ -21,6 +21,8 @@ import ( "github.com/openshift/api/config" configv1 "github.com/openshift/api/config/v1" "github.com/openshift/library-go/pkg/manifest" + + "github.com/openshift/cluster-version-operator/lib/resourcedelete" ) // Render renders critical manifests from /manifests to outputDir. @@ -148,6 +150,10 @@ func renderDir(renderConfig manifestRenderConfig, idir, odir string, overrides [ klog.Infof("excluding %s because we do not render that group/kind", manifest.String()) } else if err := manifest.Include(nil, requiredFeatureSet, clusterProfile, nil, overrides, enabledFeatureGates, majorVersion); err != nil { klog.Infof("excluding %s: %v", manifest.String(), err) + } else if found, err := resourcedelete.ValidDeleteAnnotation(manifest.Obj.GetAnnotations()); err != nil { + errs = append(errs, fmt.Errorf("invalid delete annotation in %s from %s: %w", manifest.String(), file.Name(), err)) + } else if found { + klog.Infof("excluding %s because we do not render manifests with the delete annotation", manifest.String()) } else { filteredManifests = append(filteredManifests, string(manifest.Raw)) klog.Infof("including %s", manifest.String()) diff --git a/pkg/payload/render_test.go b/pkg/payload/render_test.go index 10882dd798..d3e69039af 100644 --- a/pkg/payload/render_test.go +++ b/pkg/payload/render_test.go @@ -65,13 +65,14 @@ func TestRenderManifest(t *testing.T) { } } -func TestRenderDirWithMajorVersionFiltering(t *testing.T) { +func TestRenderDirFiltering(t *testing.T) { tests := []struct { name string manifests []testManifest majorVersion *uint64 expectedInclusions []string // Names of manifests that should be included expectedExclusions []string // Names of manifests that should be excluded + expectError bool // Whether an error is expected }{ { name: "major version 4 includes version 4 manifests", @@ -183,6 +184,68 @@ func TestRenderDirWithMajorVersionFiltering(t *testing.T) { expectedInclusions: []string{"version4-or-5-manifest", "exclude-version3-manifest"}, expectedExclusions: []string{"version6-only-manifest"}, }, + { + name: "delete annotation excludes manifests", + manifests: []testManifest{ + { + name: "normal-manifest", + annotations: map[string]string{}, + }, + { + name: "deleted-manifest", + annotations: map[string]string{ + "release.openshift.io/delete": "true", + }, + }, + { + name: "another-normal-manifest", + annotations: map[string]string{ + "some.other.annotation": "value", + }, + }, + }, + majorVersion: ptr.To(uint64(4)), + expectedInclusions: []string{"normal-manifest", "another-normal-manifest"}, + expectedExclusions: []string{"deleted-manifest"}, + }, + { + name: "delete annotation set to false returns error", + manifests: []testManifest{ + { + name: "normal-manifest", + annotations: map[string]string{}, + }, + { + name: "false-delete-manifest", + annotations: map[string]string{ + "release.openshift.io/delete": "false", + }, + }, + }, + majorVersion: ptr.To(uint64(4)), + expectedInclusions: []string{"normal-manifest"}, + expectedExclusions: []string{"false-delete-manifest"}, + expectError: true, + }, + { + name: "invalid delete annotation value returns error", + manifests: []testManifest{ + { + name: "normal-manifest", + annotations: map[string]string{}, + }, + { + name: "invalid-value-manifest", + annotations: map[string]string{ + "release.openshift.io/delete": "maybe", + }, + }, + }, + majorVersion: ptr.To(uint64(4)), + expectedInclusions: []string{"normal-manifest"}, + expectedExclusions: []string{"invalid-value-manifest"}, + expectError: true, + }, } for _, tt := range tests { @@ -233,8 +296,14 @@ func TestRenderDirWithMajorVersionFiltering(t *testing.T) { sets.Set[schema.GroupKind]{}, // filterGroupKind ) - if err != nil { - t.Fatalf("renderDir failed: %v", err) + if tt.expectError { + if err == nil { + t.Errorf("expected error but got none") + } + } else { + if err != nil { + t.Fatalf("renderDir failed: %v", err) + } } // Check which manifests were included in output From b7b86df066c2df6f455d7010c449ad77500af995 Mon Sep 17 00:00:00 2001 From: David Hurta Date: Wed, 1 Apr 2026 15:32:22 +0200 Subject: [PATCH 2/2] bootstrap: Add additional context of bootstrap pod manifest in comments The installer renders the CVO bootstrap manifests into its bootstrap-manifests directory [1], where bootstrap manifests of other related componenets are rendered as well by their respective bootstrap commands. The directory is then consumed by the cluster-bootstrap component [2]. The cluster-bootstrap component copies these manifests to the static Pod path of the node's kubelet [3]. As such, static Pods have some notable details, such as: > The spec of a static Pod cannot refer to other API objects > (e.g., ServiceAccount, ConfigMap, Secret, etc). [4] Mention this in the manifest to save some time for future developers. [1]: https://github.com/openshift/installer/blob/c93ae9fc74d7fa0f478fa250de2ba702f84a0a21/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template#L192 [2]: https://github.com/openshift/installer/blob/c93ae9fc74d7fa0f478fa250de2ba702f84a0a21/data/data/bootstrap/files/usr/local/bin/bootkube.sh.template#L576 [3]: https://github.com/openshift/cluster-bootstrap/blob/dc0d4a5cdaf8a7477cab584208dc99352f46efe2/pkg/start/bootstrap.go#L52-L60 [4]: https://kubernetes.io/docs/tasks/configure-pod-container/static-pod/ --- bootstrap/bootstrap-pod.yaml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/bootstrap/bootstrap-pod.yaml b/bootstrap/bootstrap-pod.yaml index 580eebd49b..1c72fd2bd6 100644 --- a/bootstrap/bootstrap-pod.yaml +++ b/bootstrap/bootstrap-pod.yaml @@ -1,3 +1,6 @@ +# This Pod manifest is deployed as a static Pod via kubelet during cluster bootstrapping. +# Static Pods are managed directly by kubelet and cannot reference API objects like +# ServiceAccount, ConfigMap, or Secret in their spec. apiVersion: v1 kind: Pod metadata: