diff --git a/deploy/helm/templates/rbac.yaml b/deploy/helm/templates/rbac.yaml index ed51c52e4..10ebe82a6 100644 --- a/deploy/helm/templates/rbac.yaml +++ b/deploy/helm/templates/rbac.yaml @@ -57,6 +57,12 @@ rules: - get - create - update + - apiGroups: + - "" + resources: + - secrets + verbs: + - delete - apiGroups: - "" resources: diff --git a/deploy/static/03-starboard-operator.clusterrole.yaml b/deploy/static/03-starboard-operator.clusterrole.yaml index a1d16d894..ad5a9bf35 100644 --- a/deploy/static/03-starboard-operator.clusterrole.yaml +++ b/deploy/static/03-starboard-operator.clusterrole.yaml @@ -35,6 +35,12 @@ rules: - get - create - update + - apiGroups: + - "" + resources: + - secrets + verbs: + - delete - apiGroups: - "" resources: diff --git a/itest/matcher/matcher.go b/itest/matcher/matcher.go index a0511af6e..5f8dfe7eb 100644 --- a/itest/matcher/matcher.go +++ b/itest/matcher/matcher.go @@ -76,7 +76,7 @@ func (m *vulnerabilityReportMatcher) Match(actual interface{}) (bool, error) { Name: m.owner.GetName(), UID: m.owner.GetUID(), Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }), }), "Report": MatchFields(IgnoreExtras, Fields{ @@ -150,7 +150,7 @@ func (m *configAuditReportMatcher) Match(actual interface{}) (bool, error) { Name: m.owner.GetName(), UID: m.owner.GetUID(), Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }), }), "Report": MatchFields(IgnoreExtras, Fields{ diff --git a/itest/matcher/matcher_test.go b/itest/matcher/matcher_test.go index 451220032..18b6839b0 100644 --- a/itest/matcher/matcher_test.go +++ b/itest/matcher/matcher_test.go @@ -49,7 +49,7 @@ func TestVulnerabilityReportMatcher(t *testing.T) { Name: "nginx-pod", UID: "56d53a84-c81b-4620-81a1-e226c35d3983", Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }, }, }, @@ -101,7 +101,7 @@ func TestConfigAuditReportMatcher(t *testing.T) { Name: "nginx-6d4cf56db6", UID: "494b2727-5d52-4057-9a9b-8b508c753fea", Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }, }, }, diff --git a/itest/starboard/starboard_cli_test.go b/itest/starboard/starboard_cli_test.go index 6827dcc4a..edab6b0a6 100644 --- a/itest/starboard/starboard_cli_test.go +++ b/itest/starboard/starboard_cli_test.go @@ -1044,7 +1044,7 @@ var _ = Describe("Starboard CLI", func() { Name: node.Name, UID: node.UID, Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }), }), "Report": MatchFields(IgnoreExtras, Fields{ diff --git a/pkg/configauditreport/builder.go b/pkg/configauditreport/builder.go index 9829a163c..e1ef87203 100644 --- a/pkg/configauditreport/builder.go +++ b/pkg/configauditreport/builder.go @@ -9,6 +9,7 @@ import ( "github.com/aquasecurity/starboard/pkg/starboard" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -27,11 +28,11 @@ func NewBuilder(scheme *runtime.Scheme) Builder { } type builder struct { - scheme *runtime.Scheme - controller metav1.Object - hash string - configHash string - result v1alpha1.ConfigAuditResult + scheme *runtime.Scheme + controller metav1.Object + podSpecHash string + pluginConfigHash string + result v1alpha1.ConfigAuditResult } func (b *builder) Controller(controller metav1.Object) Builder { @@ -40,12 +41,12 @@ func (b *builder) Controller(controller metav1.Object) Builder { } func (b *builder) PodSpecHash(hash string) Builder { - b.hash = hash + b.podSpecHash = hash return b } func (b *builder) PluginConfigHash(hash string) Builder { - b.configHash = hash + b.pluginConfigHash = hash return b } @@ -66,7 +67,7 @@ func (b *builder) reportName() (string, error) { func (b *builder) Get() (v1alpha1.ConfigAuditReport, error) { kind, err := kube.KindForObject(b.controller, b.scheme) if err != nil { - return v1alpha1.ConfigAuditReport{}, err + return v1alpha1.ConfigAuditReport{}, fmt.Errorf("getting kind for object: %w", err) } labels := map[string]string{ @@ -75,12 +76,12 @@ func (b *builder) Get() (v1alpha1.ConfigAuditReport, error) { starboard.LabelResourceNamespace: b.controller.GetNamespace(), } - if b.hash != "" { - labels[starboard.LabelPodSpecHash] = b.hash + if b.podSpecHash != "" { + labels[starboard.LabelPodSpecHash] = b.podSpecHash } - if b.configHash != "" { - labels[starboard.LabelPluginConfigHash] = b.configHash + if b.pluginConfigHash != "" { + labels[starboard.LabelPluginConfigHash] = b.pluginConfigHash } reportName, err := b.reportName() @@ -98,7 +99,16 @@ func (b *builder) Get() (v1alpha1.ConfigAuditReport, error) { } err = controllerutil.SetControllerReference(b.controller, &report, b.scheme) if err != nil { - return v1alpha1.ConfigAuditReport{}, err + return v1alpha1.ConfigAuditReport{}, fmt.Errorf("setting controller reference: %w", err) } + // The OwnerReferencesPermissionsEnforcement admission controller protects the + // access to metadata.ownerReferences[x].blockOwnerDeletion of an object, so + // that only users with "update" permission to the finalizers subresource of the + // referenced owner can change it. + // We set metadata.ownerReferences[x].blockOwnerDeletion to false so that + // additional RBAC permissions are not required when the OwnerReferencesPermissionsEnforcement + // is enabled. + // See https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement + report.OwnerReferences[0].BlockOwnerDeletion = pointer.BoolPtr(false) return report, nil } diff --git a/pkg/configauditreport/builder_test.go b/pkg/configauditreport/builder_test.go index a48ca35a5..2149fafa7 100644 --- a/pkg/configauditreport/builder_test.go +++ b/pkg/configauditreport/builder_test.go @@ -5,6 +5,7 @@ import ( "github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1" "github.com/aquasecurity/starboard/pkg/configauditreport" + "github.com/aquasecurity/starboard/pkg/starboard" "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -37,15 +38,15 @@ func TestBuilder(t *testing.T) { Kind: "ReplicaSet", Name: "some-owner", Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }, }, Labels: map[string]string{ - "starboard.resource.kind": "ReplicaSet", - "starboard.resource.name": "some-owner", - "starboard.resource.namespace": "qa", - "pod-spec-hash": "xyz", - "plugin-config-hash": "nop", + starboard.LabelResourceKind: "ReplicaSet", + starboard.LabelResourceName: "some-owner", + starboard.LabelResourceNamespace: "qa", + starboard.LabelPodSpecHash: "xyz", + starboard.LabelPluginConfigHash: "nop", }, }, Report: v1alpha1.ConfigAuditResult{}, diff --git a/pkg/kubebench/builder.go b/pkg/kubebench/builder.go new file mode 100644 index 000000000..04f5d728f --- /dev/null +++ b/pkg/kubebench/builder.go @@ -0,0 +1,82 @@ +package kubebench + +import ( + "fmt" + + "github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1" + "github.com/aquasecurity/starboard/pkg/kube" + "github.com/aquasecurity/starboard/pkg/starboard" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" +) + +type Builder interface { + Controller(controller metav1.Object) Builder + Data(data v1alpha1.CISKubeBenchOutput) Builder + Get() (v1alpha1.CISKubeBenchReport, error) +} + +func NewBuilder(scheme *runtime.Scheme) Builder { + return &builder{ + scheme: scheme, + } +} + +type builder struct { + scheme *runtime.Scheme + controller metav1.Object + data v1alpha1.CISKubeBenchOutput +} + +func (b *builder) Controller(controller metav1.Object) Builder { + b.controller = controller + return b +} + +func (b *builder) Data(data v1alpha1.CISKubeBenchOutput) Builder { + b.data = data + return b +} + +func (b *builder) reportName() string { + return b.controller.GetName() +} + +func (b *builder) Get() (v1alpha1.CISKubeBenchReport, error) { + kind, err := kube.KindForObject(b.controller, b.scheme) + if err != nil { + return v1alpha1.CISKubeBenchReport{}, fmt.Errorf("getting kind for object: %w", err) + } + + labels := map[string]string{ + starboard.LabelResourceKind: kind, + starboard.LabelResourceName: b.controller.GetName(), + } + + reportName := b.reportName() + + report := v1alpha1.CISKubeBenchReport{ + ObjectMeta: metav1.ObjectMeta{ + Name: reportName, + Namespace: b.controller.GetNamespace(), + Labels: labels, + }, + Report: b.data, + } + err = controllerutil.SetControllerReference(b.controller, &report, b.scheme) + if err != nil { + return v1alpha1.CISKubeBenchReport{}, fmt.Errorf("setting controller reference: %w", err) + } + // The OwnerReferencesPermissionsEnforcement admission controller protects the + // access to metadata.ownerReferences[x].blockOwnerDeletion of an object, so + // that only users with "update" permission to the finalizers subresource of the + // referenced owner can change it. + // We set metadata.ownerReferences[x].blockOwnerDeletion to false so that + // additional RBAC permissions are not required when the OwnerReferencesPermissionsEnforcement + // is enabled. + // See https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement + report.OwnerReferences[0].BlockOwnerDeletion = pointer.BoolPtr(false) + return report, nil +} diff --git a/pkg/kubebench/builder_test.go b/pkg/kubebench/builder_test.go new file mode 100644 index 000000000..f09e99426 --- /dev/null +++ b/pkg/kubebench/builder_test.go @@ -0,0 +1,47 @@ +package kubebench_test + +import ( + "testing" + + "github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1" + "github.com/aquasecurity/starboard/pkg/kubebench" + "github.com/aquasecurity/starboard/pkg/starboard" + "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/utils/pointer" +) + +func TestBuilder(t *testing.T) { + g := gomega.NewGomegaWithT(t) + + report, err := kubebench.NewBuilder(scheme.Scheme). + Controller(&corev1.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "control-plane", + }, + }). + Data(v1alpha1.CISKubeBenchOutput{}).Get() + + g.Expect(err).ToNot(gomega.HaveOccurred()) + g.Expect(report).To(gomega.Equal(v1alpha1.CISKubeBenchReport{ + ObjectMeta: metav1.ObjectMeta{ + Name: "control-plane", + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "v1", + Kind: "Node", + Name: "control-plane", + Controller: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), + }, + }, + Labels: map[string]string{ + starboard.LabelResourceKind: "Node", + starboard.LabelResourceName: "control-plane", + }, + }, + Report: v1alpha1.CISKubeBenchOutput{}, + })) +} diff --git a/pkg/kubebench/plugin_test.go b/pkg/kubebench/plugin_test.go new file mode 100644 index 000000000..4497a0c4b --- /dev/null +++ b/pkg/kubebench/plugin_test.go @@ -0,0 +1 @@ +package kubebench_test diff --git a/pkg/kubebench/scanner.go b/pkg/kubebench/scanner.go index 147a8f685..b2ae70622 100644 --- a/pkg/kubebench/scanner.go +++ b/pkg/kubebench/scanner.go @@ -21,7 +21,6 @@ import ( "k8s.io/client-go/kubernetes" "k8s.io/klog" "k8s.io/utils/pointer" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) type Scanner struct { @@ -91,20 +90,12 @@ func (s *Scanner) Scan(ctx context.Context, node corev1.Node) (v1alpha1.CISKubeB return v1alpha1.CISKubeBenchReport{}, err } - report := v1alpha1.CISKubeBenchReport{ - ObjectMeta: metav1.ObjectMeta{ - Name: node.Name, - Labels: map[string]string{ - starboard.LabelResourceKind: string(kube.KindNode), - starboard.LabelResourceName: node.Name, - }, - }, - Report: output, - } - - err = controllerutil.SetControllerReference(&node, &report, s.scheme) + report, err := NewBuilder(s.scheme). + Controller(&node). + Data(output). + Get() if err != nil { - return v1alpha1.CISKubeBenchReport{}, err + return v1alpha1.CISKubeBenchReport{}, fmt.Errorf("building report: %w", err) } return report, nil diff --git a/pkg/operator/controller/ciskubebenchreport.go b/pkg/operator/controller/ciskubebenchreport.go index e8f77457e..504cd8c24 100644 --- a/pkg/operator/controller/ciskubebenchreport.go +++ b/pkg/operator/controller/ciskubebenchreport.go @@ -22,7 +22,6 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" - "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -271,21 +270,12 @@ func (r *CISKubeBenchReportReconciler) processCompleteScanJob(ctx context.Contex _ = logsStream.Close() }() - // TODO We have a similar code in CLI - report := v1alpha1.CISKubeBenchReport{ - ObjectMeta: metav1.ObjectMeta{ - Name: node.Name, - Labels: map[string]string{ - starboard.LabelResourceKind: string(kube.KindNode), - starboard.LabelResourceName: node.Name, - }, - }, - Report: output, - } - - err = controllerutil.SetControllerReference(node, &report, r.Client.Scheme()) + report, err := kubebench.NewBuilder(r.Client.Scheme()). + Controller(node). + Data(output). + Get() if err != nil { - return fmt.Errorf("setting controller reference: %w", err) + return fmt.Errorf("building report: %w", err) } log.V(1).Info("Writing CIS Kubernetes Benchmark report", "reportName", report.Name) diff --git a/pkg/vulnerabilityreport/builder.go b/pkg/vulnerabilityreport/builder.go index fe5df6cf5..c4c6f8f8e 100644 --- a/pkg/vulnerabilityreport/builder.go +++ b/pkg/vulnerabilityreport/builder.go @@ -9,6 +9,7 @@ import ( "github.com/aquasecurity/starboard/pkg/starboard" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "k8s.io/utils/pointer" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" ) @@ -66,7 +67,7 @@ func (b *builder) reportName() (string, error) { func (b *builder) Get() (v1alpha1.VulnerabilityReport, error) { kind, err := kube.KindForObject(b.owner, b.scheme) if err != nil { - return v1alpha1.VulnerabilityReport{}, err + return v1alpha1.VulnerabilityReport{}, fmt.Errorf("getting kind for object: %w", err) } labels := map[string]string{ @@ -95,7 +96,16 @@ func (b *builder) Get() (v1alpha1.VulnerabilityReport, error) { } err = controllerutil.SetControllerReference(b.owner, &report, b.scheme) if err != nil { - return v1alpha1.VulnerabilityReport{}, err + return v1alpha1.VulnerabilityReport{}, fmt.Errorf("setting controller reference: %w", err) } + // The OwnerReferencesPermissionsEnforcement admission controller protects the + // access to metadata.ownerReferences[x].blockOwnerDeletion of an object, so + // that only users with "update" permission to the finalizers subresource of the + // referenced owner can change it. + // We set metadata.ownerReferences[x].blockOwnerDeletion to false so that + // additional RBAC permissions are not required when the OwnerReferencesPermissionsEnforcement + // is enabled. + // See https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/#ownerreferencespermissionenforcement + report.OwnerReferences[0].BlockOwnerDeletion = pointer.BoolPtr(false) return report, nil } diff --git a/pkg/vulnerabilityreport/builder_test.go b/pkg/vulnerabilityreport/builder_test.go index 3d735f1a9..3f2bfeb8f 100644 --- a/pkg/vulnerabilityreport/builder_test.go +++ b/pkg/vulnerabilityreport/builder_test.go @@ -4,6 +4,7 @@ import ( "testing" "github.com/aquasecurity/starboard/pkg/apis/aquasecurity/v1alpha1" + "github.com/aquasecurity/starboard/pkg/starboard" "github.com/aquasecurity/starboard/pkg/vulnerabilityreport" "github.com/onsi/gomega" appsv1 "k8s.io/api/apps/v1" @@ -36,15 +37,15 @@ func TestBuilder(t *testing.T) { Kind: "ReplicaSet", Name: "some-owner", Controller: pointer.BoolPtr(true), - BlockOwnerDeletion: pointer.BoolPtr(true), + BlockOwnerDeletion: pointer.BoolPtr(false), }, }, Labels: map[string]string{ - "starboard.resource.kind": "ReplicaSet", - "starboard.resource.name": "some-owner", - "starboard.resource.namespace": "qa", - "starboard.container.name": "my-container", - "pod-spec-hash": "xyz", + starboard.LabelResourceKind: "ReplicaSet", + starboard.LabelResourceName: "some-owner", + starboard.LabelResourceNamespace: "qa", + starboard.LabelContainerName: "my-container", + starboard.LabelPodSpecHash: "xyz", }, }, Report: v1alpha1.VulnerabilityScanResult{},