Skip to content

Commit

Permalink
Don't add security context on <4.11 as OpenShift restricted SCCs do …
Browse files Browse the repository at this point in the history
…not tolerate it (#3401)

* Don't add security context on <4.11 as OpenShift restricted SCCs do not
tolerate it

* Update GetClusterVersion to return cv.Status.History[0] if no completed update exists
  • Loading branch information
bennerv authored Feb 19, 2024
1 parent f4d03fb commit 5ac65fd
Show file tree
Hide file tree
Showing 16 changed files with 515 additions and 46 deletions.
2 changes: 2 additions & 0 deletions pkg/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
extensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"sigs.k8s.io/controller-runtime/pkg/client"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/cluster/graph"
Expand Down Expand Up @@ -89,6 +90,7 @@ type manager struct {
subnet subnet.Manager
graph graph.Manager

client client.Client
kubernetescli kubernetes.Interface
dynamiccli dynamic.Interface
extensionscli extensionsclient.Interface
Expand Down
16 changes: 15 additions & 1 deletion pkg/cluster/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import (
extensionsclient "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
"k8s.io/client-go/dynamic"
"k8s.io/client-go/kubernetes"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/apiutil"

"github.com/Azure/ARO-RP/pkg/api"
"github.com/Azure/ARO-RP/pkg/containerinstall"
Expand Down Expand Up @@ -499,13 +501,25 @@ func (m *manager) initializeKubernetesClients(ctx context.Context) error {
}

m.imageregistrycli, err = imageregistryclient.NewForConfig(restConfig)
if err != nil {
return err
}

mapper, err := apiutil.NewDynamicRESTMapper(restConfig, apiutil.WithLazyDiscovery)
if err != nil {
return err
}

m.client, err = client.New(restConfig, client.Options{
Mapper: mapper,
})
return err
}

// initializeKubernetesClients initializes clients which are used
// once the cluster is up later on in the install process.
func (m *manager) initializeOperatorDeployer(ctx context.Context) (err error) {
m.aroOperatorDeployer, err = deploy.New(m.log, m.env, m.doc.OpenShiftCluster, m.arocli, m.extensionscli, m.kubernetescli)
m.aroOperatorDeployer, err = deploy.New(m.log, m.env, m.doc.OpenShiftCluster, m.arocli, m.client, m.extensionscli, m.kubernetescli)
return
}

Expand Down
23 changes: 22 additions & 1 deletion pkg/operator/controllers/genevalogging/genevalogging.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
kruntime "k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/types"

pkgoperator "github.com/Azure/ARO-RP/pkg/operator"
arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1"
"github.com/Azure/ARO-RP/pkg/util/version"
)
Expand All @@ -43,6 +44,21 @@ func (r *Reconciler) securityContextConstraints(ctx context.Context, name, servi
return scc, nil
}

// namespaceLabels adds proper namespace labels for the privileged geneva logging
// daemonset on OpenShift 4.11+
func (r *Reconciler) namespaceLabels(ctx context.Context) (map[string]string, error) {
usePodSecurityAdmission, err := pkgoperator.ShouldUsePodSecurityStandard(ctx, r.Client)
if err != nil {
return nil, err
}

if usePodSecurityAdmission {
return privilegedNamespaceLabels, nil
}

return map[string]string{}, nil
}

func (r *Reconciler) daemonset(cluster *arov1alpha1.Cluster) (*appsv1.DaemonSet, error) {
resourceID, err := azure.ParseResourceID(cluster.Spec.ResourceID)
if err != nil {
Expand Down Expand Up @@ -285,12 +301,17 @@ func (r *Reconciler) resources(ctx context.Context, cluster *arov1alpha1.Cluster
return nil, err
}

nsLabels, err := r.namespaceLabels(ctx)
if err != nil {
return nil, err
}

return []kruntime.Object{
&corev1.Namespace{
ObjectMeta: metav1.ObjectMeta{
Name: kubeNamespace,
Annotations: map[string]string{projectv1.ProjectNodeSelector: ""},
Labels: privilegedNamespaceLabels,
Labels: nsLabels,
},
},
&corev1.Secret{
Expand Down
78 changes: 77 additions & 1 deletion pkg/operator/controllers/genevalogging/genevalogging_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import (
"context"
"errors"
"fmt"
"reflect"
"strings"
"testing"

"github.com/go-test/deep"
"github.com/golang/mock/gomock"
configv1 "github.com/openshift/api/config/v1"
operatorv1 "github.com/openshift/api/operator/v1"
securityv1 "github.com/openshift/api/security/v1"
"github.com/sirupsen/logrus"
Expand Down Expand Up @@ -43,6 +45,77 @@ func getContainer(d *appsv1.DaemonSet, containerName string) (corev1.Container,
return corev1.Container{}, errors.New("not found")
}

func clusterVersion(version string) configv1.ClusterVersion {
return configv1.ClusterVersion{
ObjectMeta: metav1.ObjectMeta{
Name: "version",
},
Spec: configv1.ClusterVersionSpec{},
Status: configv1.ClusterVersionStatus{
History: []configv1.UpdateHistory{
{
State: configv1.CompletedUpdate,
Version: version,
},
},
},
}
}

func TestGenevaLoggingNamespaceLabels(t *testing.T) {
tests := []struct {
name string
cv configv1.ClusterVersion
wantLabels map[string]string
wantErr string
}{
{
name: "cluster < 4.11, no labels",
cv: clusterVersion("4.10.99"),
wantLabels: map[string]string{},
},
{
name: "cluster >= 4.11, use pod security labels",
cv: clusterVersion("4.11.0"),
wantLabels: privilegedNamespaceLabels,
},
{
name: "cluster version doesn't exist",
cv: configv1.ClusterVersion{},
wantErr: `clusterversions.config.openshift.io "version" not found`,
},
{
name: "invalid version",
cv: clusterVersion("abcd"),
wantErr: `could not parse version "abcd"`,
},
}
for _, tt := range tests {
ctx := context.Background()

controller := gomock.NewController(t)
defer controller.Finish()

mockDh := mock_dynamichelper.NewMockInterface(controller)

r := &Reconciler{
AROController: base.AROController{
Log: logrus.NewEntry(logrus.StandardLogger()),
Client: ctrlfake.NewClientBuilder().WithObjects(&tt.cv).Build(),
Name: ControllerName,
},
dh: mockDh,
}

labels, err := r.namespaceLabels(ctx)
utilerror.AssertErrorMessage(t, err, tt.wantErr)

if !reflect.DeepEqual(labels, tt.wantLabels) {
t.Errorf("got: %v\nwanted:%v\n", labels, tt.wantLabels)
}
}
}

func TestGenevaLoggingDaemonset(t *testing.T) {
nominalMocks := func(mockDh *mock_dynamichelper.MockInterface) {
mockDh.EXPECT().Ensure(
Expand Down Expand Up @@ -201,6 +274,7 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
},
}

cv := clusterVersion("4.11.0")
resources := []client.Object{
instance,
&corev1.Secret{
Expand All @@ -218,6 +292,7 @@ func TestGenevaLoggingDaemonset(t *testing.T) {
Name: "privileged",
},
},
&cv,
}

mockDh := mock_dynamichelper.NewMockInterface(controller)
Expand Down Expand Up @@ -307,10 +382,11 @@ func TestGenevaConfigMapResources(t *testing.T) {
ObjectMeta: metav1.ObjectMeta{Name: "privileged"},
}

cv := clusterVersion("4.11.0")
r := &Reconciler{
AROController: base.AROController{
Log: logrus.NewEntry(logrus.StandardLogger()),
Client: ctrlfake.NewClientBuilder().WithObjects(instance, scc).Build(),
Client: ctrlfake.NewClientBuilder().WithObjects(instance, scc, &cv).Build(),
Name: ControllerName,
},
}
Expand Down
7 changes: 4 additions & 3 deletions pkg/operator/controllers/muo/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ package config
// Licensed under the Apache License 2.0.

type MUODeploymentConfig struct {
Pullspec string
EnableConnected bool
OCMBaseURL string
EnableConnected bool
OCMBaseURL string
Pullspec string
SupportsPodSecurityAdmission bool
}
10 changes: 8 additions & 2 deletions pkg/operator/controllers/muo/muo_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
pullSpec = version.MUOImage(instance.Spec.ACRDomain)
}

usePodSecurityAdmission, err := operator.ShouldUsePodSecurityStandard(ctx, r.client)
if err != nil {
return reconcile.Result{}, err
}

config := &config.MUODeploymentConfig{
Pullspec: pullSpec,
SupportsPodSecurityAdmission: usePodSecurityAdmission,
Pullspec: pullSpec,
}

disableOCM := instance.Spec.OperatorFlags.GetSimpleBoolean(controllerForceLocalOnly)
Expand Down Expand Up @@ -146,7 +152,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
timeoutCtx, cancel := context.WithTimeout(ctx, r.readinessTimeout)
defer cancel()

err := wait.PollImmediateUntil(r.readinessPollTime, func() (bool, error) {
err = wait.PollImmediateUntil(r.readinessPollTime, func() (bool, error) {
return r.deployer.IsReady(ctx, "openshift-managed-upgrade-operator", "managed-upgrade-operator")
}, timeoutCtx.Done())
if err != nil {
Expand Down
Loading

0 comments on commit 5ac65fd

Please sign in to comment.