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

Author: DavidHurta

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 {

…r-version-operator_03_roles-default.yaml …r-version-operator_90_roles-default.yamlinstall/0000_00_cluster-version-operator_03_roles-default.yaml renamed to install/0000_00_cluster-version-operator_90_roles-default.yaml install/0000_00_cluster-version-operator_03_roles-default.yaml renamed to 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

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())

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