Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

controllers: create SCCs on Openshift #52

Merged
merged 3 commits into from
Dec 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions config/rbac/kustomization.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,6 @@ resources:
- topolvm_node_service_account.yaml
- topolvm_node_role.yaml
- topolvm_node_role_bindings.yaml
# topolvm-node scc
- topolvm_node_scc.yaml
- topolvm_node_scc_role.yaml
- topolvm_node_scc_role_bindings.yaml
# Comment the following 4 lines if you want to disable
# the auth proxy (https://github.com/brancz/kube-rbac-proxy)
# which protects your /metrics endpoint.
Expand All @@ -31,4 +27,3 @@ resources:
- vg_manager_role.yaml
- vg_manager_role_binding.yaml
- vg_manager_service_account.yaml
- vg_manager_scc.yaml
9 changes: 9 additions & 0 deletions config/rbac/role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,15 @@ rules:
- get
- patch
- update
- apiGroups:
- security.openshift.io
resources:
- securitycontextconstraints
verbs:
- create
- delete
- get
- update
- apiGroups:
- storage.k8s.io
resources:
Expand Down
29 changes: 0 additions & 29 deletions config/rbac/topolvm_node_scc.yaml

This file was deleted.

13 changes: 0 additions & 13 deletions config/rbac/topolvm_node_scc_role.yaml

This file was deleted.

12 changes: 0 additions & 12 deletions config/rbac/topolvm_node_scc_role_bindings.yaml

This file was deleted.

8 changes: 0 additions & 8 deletions config/rbac/vg_manager_role.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,3 @@ rules:
- get
- patch
- update
- apiGroups:
- security.openshift.io
resourceNames:
- vg-manager
resources:
- securitycontextconstraints
verbs:
- use
52 changes: 49 additions & 3 deletions controllers/lvmcluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ import (
"fmt"

"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

"github.com/go-logr/logr"
secv1client "github.com/openshift/client-go/security/clientset/versioned/typed/security/v1"
lvmv1alpha1 "github.com/red-hat-storage/lvm-operator/api/v1alpha1"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
Expand All @@ -36,19 +38,31 @@ var lvmClusterFinalizer = "lvmcluster.topolvm.io"

const (
ControllerName = "lvmcluster-controller"

openshiftSCCPrivilegedName = "privileged"
)

type ClusterType string

const (
ClusterTypeOCP ClusterType = "openshift"
ClusterTypeOther ClusterType = "other"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really need other option here? I think only ClusterTypeOCP should do the job.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

other looks a bit ambiguous as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is set to a value to indicate that the check has been performed and a result has been received.

)

// LVMClusterReconciler reconciles a LVMCluster object
type LVMClusterReconciler struct {
client.Client
Scheme *runtime.Scheme
Log logr.Logger
Scheme *runtime.Scheme
Log logr.Logger
ClusterType ClusterType
SecurityClient secv1client.SecurityV1Interface
}

//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=apps,resources=daemonsets,verbs=get;list;watch;create;update;patch;delete
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters/status,verbs=get;update;patch
//+kubebuilder:rbac:groups=lvm.topolvm.io,resources=lvmclusters/finalizers,verbs=update
//+kubebuilder:rbac:groups=security.openshift.io,resources=securitycontextconstraints,verbs=get;create;update;delete

// Reconcile is part of the main kubernetes reconciliation loop which aims to
// move the current state of the cluster closer to the desired state.
Expand All @@ -74,7 +88,11 @@ func (r *LVMClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// Error reading the object - requeue the request.
return ctrl.Result{}, err
}

err = r.checkIfOpenshift(ctx)
if err != nil {
r.Log.Error(err, "failed to check cluster type")
return ctrl.Result{}, err
}
result, reconcileError := r.reconcile(ctx, lvmCluster)

// Apply status changes
Expand All @@ -100,6 +118,7 @@ func (r *LVMClusterReconciler) reconcile(ctx context.Context, instance *lvmv1alp
resourceList := []resourceManager{
&csiDriver{},
&topolvmController{},
&openshiftSccs{},
&topolvmNode{},
&vgManager{},
&topolvmStorageClass{},
Expand Down Expand Up @@ -179,3 +198,30 @@ type resourceManager interface {
// status that changes only when the operands change.
updateStatus(*LVMClusterReconciler, context.Context, *lvmv1alpha1.LVMCluster) error
}

// checkIfOpenshift checks to see if the operator is running on an OCP cluster.
// It does this by querying for the "privileged" SCC which exists on all OCP clusters.
func (r *LVMClusterReconciler) checkIfOpenshift(ctx context.Context) error {
if r.ClusterType == "" {
// cluster type has not been determined yet
// Check if the privileged SCC exists on the cluster (this is one of the default SCCs)
_, err := r.SecurityClient.SecurityContextConstraints().Get(ctx, openshiftSCCPrivilegedName, metav1.GetOptions{})
if err != nil {
if errors.IsNotFound(err) {
// Not an Openshift cluster
r.ClusterType = ClusterTypeOther
} else {
// Something went wrong
r.Log.Error(err, "failed to get SCC", "Name", openshiftSCCPrivilegedName)
return err
}
} else {
r.ClusterType = ClusterTypeOCP
}
}
return nil
}

func IsOpenshift(r *LVMClusterReconciler) bool {
return r.ClusterType == ClusterTypeOCP
}
180 changes: 180 additions & 0 deletions controllers/scc.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,180 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • please add this as one of resource unit, just reminding

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Copyright 2021.

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package controllers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you try below once?

diff --git a/controllers/scc.go b/controllers/scc.go
index a987590..f0a9d62 100644
--- a/controllers/scc.go
+++ b/controllers/scc.go
@@ -45,17 +45,17 @@ func (c openshiftSccs) ensureCreated(r *LVMClusterReconciler, ctx context.Contex
 
                sccs := getAllSCCs(lvmCluster.Namespace)
                for _, scc := range sccs {
-                       found, err := r.SecurityClient.SecurityContextConstraints().Get(scc.Name, metav1.GetOptions{})
+                       found, err := r.SecurityClient.SecurityContextConstraints().Get(ctx, scc.Name, metav1.GetOptions{})
                        if err != nil && errors.IsNotFound(err) {
                                r.Log.Info("Creating SecurityContextConstraint.", "SecurityContextConstraint", scc.Name)
-                               _, err := r.SecurityClient.SecurityContextConstraints().Create(scc)
+                               _, err := r.SecurityClient.SecurityContextConstraints().Create(ctx, scc, metav1.CreateOptions{})
                                if err != nil {
                                        return fmt.Errorf("unable to create SCC %+v: %v", scc, err)
                                }
                        } else if err == nil {
                                scc.ObjectMeta = found.ObjectMeta
                                r.Log.Info("Updating SecurityContextConstraint.", "SecurityContextConstraint", scc.Name)
-                               _, err := r.SecurityClient.SecurityContextConstraints().Update(scc)
+                               _, err := r.SecurityClient.SecurityContextConstraints().Update(ctx, scc, metav1.UpdateOptions{})
                                if err != nil {
                                        r.Log.Error(err, "Unable to update SecurityContextConstraint.", "SecurityContextConstraint", scc.Name)
                                        return fmt.Errorf("unable to update SCC %+v: %v", scc, err)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was what I had originally but that did not match the v3.9 signatures

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does work with the updated packages .


import (
"context"
"fmt"

secv1 "github.com/openshift/api/security/v1"
lvmv1alpha1 "github.com/red-hat-storage/lvm-operator/api/v1alpha1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

const (
sccName = "topolvm-scc"
)

type openshiftSccs struct{}

// openshiftSccs unit satisfies resourceManager interface
var _ resourceManager = openshiftSccs{}

func (c openshiftSccs) getName() string {
return sccName
}

func (c openshiftSccs) ensureCreated(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
if !IsOpenshift(r) {
r.Log.Info("not creating SCCs as this is not an Openshift cluster")
return nil
}
sccs := getAllSCCs(lvmCluster.Namespace)
for _, scc := range sccs {
_, err := r.SecurityClient.SecurityContextConstraints().Get(ctx, scc.Name, metav1.GetOptions{})
if err != nil && errors.IsNotFound(err) {
r.Log.Info("creating SecurityContextConstraint", "SecurityContextConstraint", scc.Name)
_, err := r.SecurityClient.SecurityContextConstraints().Create(ctx, scc, metav1.CreateOptions{})
if err != nil {
return fmt.Errorf("failed to create SCC %q: %v", scc.Name, err)
}
} else if err == nil {
// Don't update the SCC
r.Log.Info("already exists", "SecurityContextConstraint", scc.Name)
} else {
r.Log.Error(err, "Something went wrong when checking for SecurityContextConstraint", "SecurityContextConstraint", scc.Name)
return fmt.Errorf("something went wrong when checking for SCC %q: %v", scc.Name, err)
}
}

return nil
}

func (c openshiftSccs) ensureDeleted(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
if IsOpenshift(r) {
var err error
sccs := getAllSCCs(lvmCluster.Namespace)
for _, scc := range sccs {
err = r.SecurityClient.SecurityContextConstraints().Delete(ctx, scc.Name, metav1.DeleteOptions{})
if err != nil {
if errors.IsNotFound(err) {
r.Log.Info("SecurityContextConstraint is already deleted", "SecurityContextConstraint", scc.Name)
return nil
} else {
r.Log.Error(err, "failed to delete SecurityContextConstraint", "SecurityContextConstraint", scc.Name)
}
}
}
}
return nil
}

func (c openshiftSccs) updateStatus(r *LVMClusterReconciler, ctx context.Context, lvmCluster *lvmv1alpha1.LVMCluster) error {
// intentionally empty
return nil
}

func getAllSCCs(namespace string) []*secv1.SecurityContextConstraints {
return []*secv1.SecurityContextConstraints{
newTopolvmNodeScc(namespace),
newVGManagerScc(namespace),
}
}

func newTopolvmNodeScc(namespace string) *secv1.SecurityContextConstraints {
scc := &secv1.SecurityContextConstraints{
TypeMeta: metav1.TypeMeta{
APIVersion: "security.openshift.io/v1",
Kind: "SecurityContextConstraints",
},
}
scc.Name = "odf-lvm-topolvm-node"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the prefix odf intentional here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. SCCs will be created only on Openshift so Odf is fine.

scc.AllowPrivilegedContainer = true
scc.AllowHostNetwork = false
scc.AllowHostDirVolumePlugin = true
scc.AllowHostPorts = false
scc.AllowHostPID = true
scc.AllowHostIPC = false
scc.ReadOnlyRootFilesystem = false
scc.RequiredDropCapabilities = []corev1.Capability{}
scc.RunAsUser = secv1.RunAsUserStrategyOptions{
Type: secv1.RunAsUserStrategyRunAsAny,
}
scc.SELinuxContext = secv1.SELinuxContextStrategyOptions{
Type: secv1.SELinuxStrategyRunAsAny,
}
scc.FSGroup = secv1.FSGroupStrategyOptions{
Type: secv1.FSGroupStrategyRunAsAny,
}
scc.SupplementalGroups = secv1.SupplementalGroupsStrategyOptions{
Type: secv1.SupplementalGroupsStrategyRunAsAny,
}
scc.Volumes = []secv1.FSType{
secv1.FSTypeConfigMap,
secv1.FSTypeEmptyDir,
secv1.FSTypeHostPath,
secv1.FSTypeSecret,
}
scc.Users = []string{
fmt.Sprintf("system:serviceaccount:%s:%s", namespace, TopolvmNodeServiceAccount),
}

return scc
}

func newVGManagerScc(namespace string) *secv1.SecurityContextConstraints {
scc := &secv1.SecurityContextConstraints{
TypeMeta: metav1.TypeMeta{
APIVersion: "security.openshift.io/v1",
Kind: "SecurityContextConstraints",
},
}
scc.Name = "odf-lvm-vgmanager"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • intentional odf prefix?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above.

scc.AllowPrivilegedContainer = true
scc.AllowHostNetwork = false
scc.AllowHostDirVolumePlugin = true
scc.AllowHostPorts = false
scc.AllowHostPID = true
scc.AllowHostIPC = true
scc.ReadOnlyRootFilesystem = false
scc.RequiredDropCapabilities = []corev1.Capability{}
scc.RunAsUser = secv1.RunAsUserStrategyOptions{
Type: secv1.RunAsUserStrategyRunAsAny,
}
scc.SELinuxContext = secv1.SELinuxContextStrategyOptions{
Type: secv1.SELinuxStrategyMustRunAs,
}
scc.FSGroup = secv1.FSGroupStrategyOptions{
Type: secv1.FSGroupStrategyMustRunAs,
}
scc.SupplementalGroups = secv1.SupplementalGroupsStrategyOptions{
Type: secv1.SupplementalGroupsStrategyRunAsAny,
}
scc.Volumes = []secv1.FSType{
secv1.FSTypeConfigMap,
secv1.FSTypeEmptyDir,
secv1.FSTypeHostPath,
secv1.FSTypeSecret,
}
scc.Users = []string{
fmt.Sprintf("system:serviceaccount:%s:%s", namespace, VGManagerServiceAccount),
}

return scc
}
Loading