From da46ec5cd15e277ebb0317b6378764ac96e42ddf 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. [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 | 56 ++++++++++++++++++- 4 files changed, 65 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..1b22866f32 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,49 @@ 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: "invalid delete annotation value returns error but continues rendering", + manifests: []testManifest{ + { + name: "normal-manifest", + annotations: map[string]string{}, + }, + { + name: "invalid-delete-manifest", + annotations: map[string]string{ + "release.openshift.io/delete": "false", + }, + }, + }, + majorVersion: ptr.To(uint64(4)), + expectedInclusions: []string{"normal-manifest"}, + expectedExclusions: []string{"invalid-delete-manifest"}, + expectError: true, + }, } for _, tt := range tests { @@ -233,8 +277,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 5fe6918e006d44f2e24e126cb0db1a8d94bbef81 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 | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bootstrap/bootstrap-pod.yaml b/bootstrap/bootstrap-pod.yaml index 580eebd49b..c9fffbdad4 100644 --- a/bootstrap/bootstrap-pod.yaml +++ b/bootstrap/bootstrap-pod.yaml @@ -1,3 +1,5 @@ +# The Pod is deployed as a static Pod during cluster bootstrap by the installer. Static Pods have some notable details, +# such as the fact that the spec cannot refer to API objects, like ServiceAccount and ConfigMap. apiVersion: v1 kind: Pod metadata: