Skip to content

Commit

Permalink
[sync] fix: Prevent containers from running as root by removing unnec…
Browse files Browse the repository at this point in the history
…essary anyuid SCC bindings (#1590)

* Removed Dashboard ServiceAccount from default RoleBinding granting anyuid SCC.

* Removed ModelMesh ServiceAccounts from default RoleBinding granting anyuid SCC.

* Removed ModelController ServiceAccount from default RoleBinding granting anyuid SCC.

* Removed Notebook ServiceAccount from default RoleBinding granting anyuid SCC.

* Remove NewUpdatePodSecurityRoleBindingAction, its corresponding test, and the security folder containing both, as they are no longer needed.

* Remove default rolebinding and its corresponding tests.

* Clean up default RoleBinding during Operator upgrade

* Clean up RoleBinding from function doc

Clean up RoleBinding from function doc
  • Loading branch information
ugiordan authored Jan 30, 2025
1 parent 5d47007 commit c4a2e3d
Show file tree
Hide file tree
Showing 13 changed files with 5 additions and 290 deletions.
2 changes: 0 additions & 2 deletions controllers/components/dashboard/dashboard_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component"
Expand Down Expand Up @@ -91,7 +90,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
WithAction(initialize).
WithAction(devFlags).
WithAction(configureDependencies).
WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).
WithAction(kustomize.NewAction(
kustomize.WithCache(),
// Those are the default labels added by the legacy deploy method
Expand Down
7 changes: 0 additions & 7 deletions controllers/components/dashboard/dashboard_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,6 @@ var (
cluster.Unknown: "/odh",
}

serviceAccounts = map[cluster.Platform][]string{
cluster.SelfManagedRhoai: {"rhods-dashboard"},
cluster.ManagedRhoai: {"rhods-dashboard"},
cluster.OpenDataHub: {"odh-dashboard"},
cluster.Unknown: {"odh-dashboard"},
}

imagesMap = map[string]string{
"odh-dashboard-image": "RELATED_IMAGE_ODH_DASHBOARD_IMAGE",
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component"
Expand Down Expand Up @@ -70,7 +69,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
// Add ModelController specific actions
WithAction(initialize).
WithAction(devFlags). // devFlags triggerd by changes in DSC kserve and ModelMeshServing, also update .status.devflagurl
WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).
WithAction(kustomize.NewAction(
kustomize.WithCache(),
kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
)
Expand All @@ -25,13 +24,6 @@ var (
imageParamMap = map[string]string{
"odh-model-controller": "RELATED_IMAGE_ODH_MODEL_CONTROLLER_IMAGE",
}

serviceAccounts = map[cluster.Platform][]string{
cluster.SelfManagedRhoai: {LegacyComponentName},
cluster.ManagedRhoai: {LegacyComponentName},
cluster.OpenDataHub: {LegacyComponentName},
cluster.Unknown: {LegacyComponentName},
}
)

func manifestsPath() types.ManifestInfo {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/clusterrole"
Expand Down Expand Up @@ -86,7 +85,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
// Add ModelMeshServing specific actions
WithAction(initialize).
WithAction(devFlags).
WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).
WithAction(kustomize.NewAction(
kustomize.WithCache(),
kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (

componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
)
Expand All @@ -28,13 +27,6 @@ var (
"odh-modelmesh": "RELATED_IMAGE_ODH_MODELMESH_IMAGE",
"odh-modelmesh-controller": "RELATED_IMAGE_ODH_MODELMESH_CONTROLLER_IMAGE",
}

serviceAccounts = map[cluster.Platform][]string{
cluster.SelfManagedRhoai: {"modelmesh", "modelmesh-controller"},
cluster.ManagedRhoai: {"modelmesh", "modelmesh-controller"},
cluster.OpenDataHub: {"modelmesh", "modelmesh-controller"},
cluster.Unknown: {"modelmesh", "modelmesh-controller"},
}
)

func manifestsPath() odhtypes.ManifestInfo {
Expand Down
2 changes: 0 additions & 2 deletions controllers/components/workbenches/workbenches_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/deploy"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/gc"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/render/kustomize"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/security"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/actions/updatestatus"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/handlers"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/predicates/component"
Expand Down Expand Up @@ -62,7 +61,6 @@ func (s *componentHandler) NewComponentReconciler(ctx context.Context, mgr ctrl.
WithAction(initialize).
WithAction(devFlags).
WithAction(configureDependencies).
WithAction(security.NewUpdatePodSecurityRoleBindingAction(serviceAccounts)).
WithAction(kustomize.NewAction(
kustomize.WithCache(),
kustomize.WithLabel(labels.ODH.Component(LegacyComponentName), labels.True),
Expand Down
10 changes: 0 additions & 10 deletions controllers/components/workbenches/workbenches_support.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

componentApi "github.com/opendatahub-io/opendatahub-operator/v2/apis/components/v1alpha1"
"github.com/opendatahub-io/opendatahub-operator/v2/controllers/status"
"github.com/opendatahub-io/opendatahub-operator/v2/pkg/cluster"
odhtypes "github.com/opendatahub-io/opendatahub-operator/v2/pkg/controller/types"
odhdeploy "github.com/opendatahub-io/opendatahub-operator/v2/pkg/deploy"
)
Expand All @@ -26,8 +25,6 @@ const (
kfNotebookControllerPath = "kf-notebook-controller"
kfNotebookControllerManifestSourcePath = "overlays/openshift"

nbcServiceAccountName = "notebook-controller-service-account"

// LegacyComponentName is the name of the component that is assigned to deployments
// via Kustomize. Since a deployment selector is immutable, we can't upgrade existing
// deployment to the new component name, so keep it around till we figure out a solution.
Expand All @@ -38,13 +35,6 @@ var (
notebookControllerContextDir = path.Join(ComponentName, notebookControllerPath)
kfNotebookControllerContextDir = path.Join(ComponentName, kfNotebookControllerPath)
notebookContextDir = path.Join(ComponentName, notebooksPath)

serviceAccounts = map[cluster.Platform][]string{
cluster.SelfManagedRhoai: {nbcServiceAccountName},
cluster.ManagedRhoai: {nbcServiceAccountName},
cluster.OpenDataHub: {nbcServiceAccountName},
cluster.Unknown: {nbcServiceAccountName},
}
)

// manifests for nbc in ODH and RHOAI + downstream use it for imageparams.
Expand Down
87 changes: 0 additions & 87 deletions controllers/dscinitialization/dscinitialization_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
userv1 "github.com/openshift/api/user/v1"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
rbacv1 "k8s.io/api/rbac/v1"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"sigs.k8s.io/controller-runtime/pkg/client"
Expand Down Expand Up @@ -87,32 +86,6 @@ var _ = Describe("DataScienceCluster initialization", func() {
Expect(foundNetworkPolicy.Spec.PolicyTypes[0]).To(Equal(networkingv1.PolicyTypeIngress))
})

It("Should create default rolebinding", func(ctx context.Context) {
// then
foundRoleBinding := &rbacv1.RoleBinding{}
Eventually(objectExists(applicationNamespace, applicationNamespace, foundRoleBinding)).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())
expectedSubjects := []rbacv1.Subject{
{
Kind: "ServiceAccount",
Namespace: applicationNamespace,
Name: "default",
},
}
expectedRoleRef := rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Name: "system:openshift:scc:anyuid",
}
Expect(foundRoleBinding.Name).To(Equal(applicationNamespace))
Expect(foundRoleBinding.Namespace).To(Equal(applicationNamespace))
Expect(foundRoleBinding.Subjects).To(Equal(expectedSubjects))
Expect(foundRoleBinding.RoleRef).To(Equal(expectedRoleRef))
})

It("Should create default configmap", func(ctx context.Context) {
// then
foundConfigMap := &corev1.ConfigMap{}
Expand Down Expand Up @@ -184,54 +157,6 @@ var _ = Describe("DataScienceCluster initialization", func() {
AfterEach(cleanupResources)
const applicationName = "default-dsci"

It("Should not update rolebinding if it exists", func(ctx context.Context) {

// given
desiredRoleBinding := &rbacv1.RoleBinding{
TypeMeta: metav1.TypeMeta{
Kind: "RoleBinding",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: applicationNamespace,
Namespace: applicationNamespace,
},

RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Name: "system:openshift:scc:anyuid",
},
}
Expect(k8sClient.Create(ctx, desiredRoleBinding)).Should(Succeed())
createdRoleBinding := &rbacv1.RoleBinding{}
Eventually(objectExists(applicationNamespace, applicationNamespace, createdRoleBinding)).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())

// when
desiredDsci := createDSCI(operatorv1.Managed, operatorv1.Managed, monitoringNamespace)
Expect(k8sClient.Create(ctx, desiredDsci)).Should(Succeed())
foundDsci := &dsciv1.DSCInitialization{}
Eventually(dscInitializationIsReady(applicationName, workingNamespace, foundDsci)).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())

// then
foundRoleBinding := &rbacv1.RoleBinding{}
Eventually(objectExists(applicationNamespace, applicationNamespace, foundRoleBinding)).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())
Expect(foundRoleBinding.UID).To(Equal(createdRoleBinding.UID))
Expect(foundRoleBinding.Subjects).To(BeNil())
})

It("Should not update configmap if it exists", func(ctx context.Context) {

// given
Expand Down Expand Up @@ -388,24 +313,12 @@ func cleanupResources(ctx context.Context) {

Expect(k8sClient.DeleteAllOf(ctx, &networkingv1.NetworkPolicy{}, appNamespace)).To(Succeed())
Expect(k8sClient.DeleteAllOf(ctx, &corev1.ConfigMap{}, appNamespace)).To(Succeed())
Expect(k8sClient.DeleteAllOf(ctx, &rbacv1.RoleBinding{}, appNamespace)).To(Succeed())
Expect(k8sClient.DeleteAllOf(ctx, &rbacv1.ClusterRoleBinding{}, appNamespace)).To(Succeed())

Eventually(noInstanceExistsIn(workingNamespace, &dsciv1.DSCInitializationList{})).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())
Eventually(noInstanceExistsIn(applicationNamespace, &rbacv1.ClusterRoleBindingList{})).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())
Eventually(noInstanceExistsIn(applicationNamespace, &rbacv1.RoleBindingList{})).
WithContext(ctx).
WithTimeout(timeout).
WithPolling(interval).
Should(BeTrue())
Eventually(noInstanceExistsIn(applicationNamespace, &corev1.ConfigMapList{})).
WithContext(ctx).
WithContext(ctx).
Expand Down
59 changes: 1 addition & 58 deletions controllers/dscinitialization/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
userv1 "github.com/openshift/api/user/v1"
corev1 "k8s.io/api/core/v1"
networkingv1 "k8s.io/api/networking/v1"
rbacv1 "k8s.io/api/rbac/v1"
k8serr "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
Expand Down Expand Up @@ -43,8 +42,7 @@ var (
//
// - 2. Patch monitoring namespace
// - 3. Network Policies 'opendatahub' that allow traffic between the ODH namespaces
// - 4. ConfigMap 'odh-common-config'
// - 5. RoleBinding 'opendatahub'.
// - 4. ConfigMap 'odh-common-config'.
func (r *DSCInitializationReconciler) createOperatorResource(ctx context.Context, dscInit *dsciv1.DSCInitialization, platform cluster.Platform) error {
log := logf.FromContext(ctx)

Expand Down Expand Up @@ -74,12 +72,6 @@ func (r *DSCInitializationReconciler) createOperatorResource(ctx context.Context
return err
}

// Create default Rolebinding for the namespace
err = r.createDefaultRoleBinding(ctx, dscInit)
if err != nil {
log.Error(err, "error creating rolebinding", "name", dscInit.Spec.ApplicationsNamespace)
return err
}
return nil
}

Expand Down Expand Up @@ -177,55 +169,6 @@ func (r *DSCInitializationReconciler) patchMonitoringNS(ctx context.Context, dsc
return err
}

func (r *DSCInitializationReconciler) createDefaultRoleBinding(ctx context.Context, dscInit *dsciv1.DSCInitialization) error {
log := logf.FromContext(ctx)
name := dscInit.Spec.ApplicationsNamespace
// Expected namespace for the given name
desiredRoleBinding := &rbacv1.RoleBinding{
TypeMeta: metav1.TypeMeta{
Kind: "RoleBinding",
APIVersion: "v1",
},
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: name,
},
Subjects: []rbacv1.Subject{
{
Kind: "ServiceAccount",
Namespace: name,
Name: "default",
},
},
RoleRef: rbacv1.RoleRef{
APIGroup: "rbac.authorization.k8s.io",
Kind: "ClusterRole",
Name: "system:openshift:scc:anyuid",
},
}

// Create RoleBinding if doesn't exists
foundRoleBinding := &rbacv1.RoleBinding{}
err := r.Client.Get(ctx, client.ObjectKeyFromObject(desiredRoleBinding), foundRoleBinding)
if err != nil {
if k8serr.IsNotFound(err) {
// Set Controller reference
err = ctrl.SetControllerReference(dscInit, desiredRoleBinding, r.Scheme)
if err != nil {
log.Error(err, "Unable to add OwnerReference to the rolebinding")
return err
}
err = r.Client.Create(ctx, desiredRoleBinding)
if err != nil && !k8serr.IsAlreadyExists(err) {
return err
}
} else {
return err
}
}
return nil
}

func (r *DSCInitializationReconciler) reconcileDefaultNetworkPolicy(
ctx context.Context,
dscInit *dsciv1.DSCInitialization,
Expand Down
26 changes: 0 additions & 26 deletions pkg/controller/actions/security/actions.go

This file was deleted.

Loading

0 comments on commit c4a2e3d

Please sign in to comment.