diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index db6c8a1dff1..c2aaf589cb4 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -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" @@ -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 diff --git a/pkg/cluster/install.go b/pkg/cluster/install.go index 6e43d971b82..b56b2b0aba7 100644 --- a/pkg/cluster/install.go +++ b/pkg/cluster/install.go @@ -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" @@ -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 } diff --git a/pkg/operator/controllers/genevalogging/genevalogging.go b/pkg/operator/controllers/genevalogging/genevalogging.go index a654e4f1363..46d7368860e 100644 --- a/pkg/operator/controllers/genevalogging/genevalogging.go +++ b/pkg/operator/controllers/genevalogging/genevalogging.go @@ -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" ) @@ -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 { @@ -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{ diff --git a/pkg/operator/controllers/genevalogging/genevalogging_test.go b/pkg/operator/controllers/genevalogging/genevalogging_test.go index 22435409655..93839cddc41 100644 --- a/pkg/operator/controllers/genevalogging/genevalogging_test.go +++ b/pkg/operator/controllers/genevalogging/genevalogging_test.go @@ -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" @@ -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( @@ -201,6 +274,7 @@ func TestGenevaLoggingDaemonset(t *testing.T) { }, } + cv := clusterVersion("4.11.0") resources := []client.Object{ instance, &corev1.Secret{ @@ -218,6 +292,7 @@ func TestGenevaLoggingDaemonset(t *testing.T) { Name: "privileged", }, }, + &cv, } mockDh := mock_dynamichelper.NewMockInterface(controller) @@ -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, }, } diff --git a/pkg/operator/controllers/muo/config/config.go b/pkg/operator/controllers/muo/config/config.go index f4819502c11..9fd6e8dd11e 100644 --- a/pkg/operator/controllers/muo/config/config.go +++ b/pkg/operator/controllers/muo/config/config.go @@ -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 } diff --git a/pkg/operator/controllers/muo/muo_controller.go b/pkg/operator/controllers/muo/muo_controller.go index 81a80b208e3..4424a60ba5c 100644 --- a/pkg/operator/controllers/muo/muo_controller.go +++ b/pkg/operator/controllers/muo/muo_controller.go @@ -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) @@ -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 { diff --git a/pkg/operator/controllers/muo/muo_controller_test.go b/pkg/operator/controllers/muo/muo_controller_test.go index e45c09c46f4..2dccbbe30a7 100644 --- a/pkg/operator/controllers/muo/muo_controller_test.go +++ b/pkg/operator/controllers/muo/muo_controller_test.go @@ -10,6 +10,7 @@ import ( "time" "github.com/golang/mock/gomock" + configv1 "github.com/openshift/api/config/v1" "github.com/sirupsen/logrus" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -26,9 +27,10 @@ import ( func TestMUOReconciler(t *testing.T) { tests := []struct { - name string - mocks func(*mock_deployer.MockDeployer, *arov1alpha1.Cluster) - flags arov1alpha1.OperatorFlags + name string + mocks func(*mock_deployer.MockDeployer, *arov1alpha1.Cluster) + flags arov1alpha1.OperatorFlags + clusterVersion string // connected MUO -- cluster pullsecret pullsecret string // errors @@ -49,6 +51,7 @@ func TestMUOReconciler(t *testing.T) { operator.MuoManaged: operator.FlagTrue, controllerPullSpec: "wonderfulPullspec", }, + clusterVersion: "4.10.0", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -64,6 +67,7 @@ func TestMUOReconciler(t *testing.T) { operator.MuoEnabled: operator.FlagTrue, operator.MuoManaged: operator.FlagTrue, }, + clusterVersion: "4.10.0", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "acrtest.example.com/app-sre/managed-upgrade-operator:v0.1.952-44b631a", @@ -81,6 +85,7 @@ func TestMUOReconciler(t *testing.T) { controllerForceLocalOnly: "false", controllerPullSpec: "wonderfulPullspec", }, + clusterVersion: "4.10.0", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -98,7 +103,8 @@ func TestMUOReconciler(t *testing.T) { controllerForceLocalOnly: "false", controllerPullSpec: "wonderfulPullspec", }, - pullsecret: "{\"auths\": {}}", + clusterVersion: "4.10.0", + pullsecret: "{\"auths\": {}}", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -116,7 +122,8 @@ func TestMUOReconciler(t *testing.T) { controllerForceLocalOnly: "false", controllerPullSpec: "wonderfulPullspec", }, - pullsecret: "i'm a little json, short and stout", + clusterVersion: "4.10.0", + pullsecret: "i'm a little json, short and stout", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -134,7 +141,8 @@ func TestMUOReconciler(t *testing.T) { controllerForceLocalOnly: "false", controllerPullSpec: "wonderfulPullspec", }, - pullsecret: "{\"auths\": {\"" + pullSecretOCMKey + "\": {\"auth\": \"secret value\"}}}", + clusterVersion: "4.10.0", + pullsecret: "{\"auths\": {\"" + pullSecretOCMKey + "\": {\"auth\": \"secret value\"}}}", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -154,7 +162,8 @@ func TestMUOReconciler(t *testing.T) { controllerOcmBaseURL: "https://example.com", controllerPullSpec: "wonderfulPullspec", }, - pullsecret: "{\"auths\": {\"" + pullSecretOCMKey + "\": {\"auth\": \"secret value\"}}}", + clusterVersion: "4.10.0", + pullsecret: "{\"auths\": {\"" + pullSecretOCMKey + "\": {\"auth\": \"secret value\"}}}", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -173,7 +182,8 @@ func TestMUOReconciler(t *testing.T) { controllerForceLocalOnly: "true", controllerPullSpec: "wonderfulPullspec", }, - pullsecret: "{\"auths\": {\"" + pullSecretOCMKey + "\": {\"auth\": \"secret value\"}}}", + clusterVersion: "4.10.0", + pullsecret: "{\"auths\": {\"" + pullSecretOCMKey + "\": {\"auth\": \"secret value\"}}}", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ Pullspec: "wonderfulPullspec", @@ -190,16 +200,27 @@ func TestMUOReconciler(t *testing.T) { operator.MuoManaged: operator.FlagTrue, controllerPullSpec: "wonderfulPullspec", }, + clusterVersion: "4.11.0", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { expectedConfig := &config.MUODeploymentConfig{ - Pullspec: "wonderfulPullspec", - EnableConnected: false, + Pullspec: "wonderfulPullspec", + EnableConnected: false, + SupportsPodSecurityAdmission: true, } md.EXPECT().CreateOrUpdate(gomock.Any(), cluster, expectedConfig).Return(nil) md.EXPECT().IsReady(gomock.Any(), gomock.Any(), gomock.Any()).Return(false, nil) }, wantErr: "managed Upgrade Operator deployment timed out on Ready: timed out waiting for the condition", }, + { + name: "managed, could not parse cluster version fails", + flags: arov1alpha1.OperatorFlags{ + operator.MuoEnabled: operator.FlagTrue, + operator.MuoManaged: operator.FlagTrue, + controllerPullSpec: "wonderfulPullspec", + }, + wantErr: `could not parse version ""`, + }, { name: "managed, CreateOrUpdate() fails", flags: arov1alpha1.OperatorFlags{ @@ -207,6 +228,7 @@ func TestMUOReconciler(t *testing.T) { operator.MuoManaged: operator.FlagTrue, controllerPullSpec: "wonderfulPullspec", }, + clusterVersion: "4.10.0", mocks: func(md *mock_deployer.MockDeployer, cluster *arov1alpha1.Cluster) { md.EXPECT().CreateOrUpdate(gomock.Any(), cluster, gomock.AssignableToTypeOf(&config.MUODeploymentConfig{})).Return(errors.New("failed ensure")) }, @@ -263,8 +285,24 @@ func TestMUOReconciler(t *testing.T) { ACRDomain: "acrtest.example.com", }, } + + cv := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{}, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + { + State: configv1.CompletedUpdate, + Version: tt.clusterVersion, + }, + }, + }, + } + deployer := mock_deployer.NewMockDeployer(controller) - clientBuilder := ctrlfake.NewClientBuilder().WithObjects(instance) + clientBuilder := ctrlfake.NewClientBuilder().WithObjects(instance, cv) if tt.pullsecret != "" { clientBuilder = clientBuilder.WithObjects(&corev1.Secret{ diff --git a/pkg/operator/controllers/muo/staticresources/deployment.yaml b/pkg/operator/controllers/muo/staticresources/deployment.yaml.tmpl similarity index 94% rename from pkg/operator/controllers/muo/staticresources/deployment.yaml rename to pkg/operator/controllers/muo/staticresources/deployment.yaml.tmpl index d6fecb58eee..a868c8acf0b 100644 --- a/pkg/operator/controllers/muo/staticresources/deployment.yaml +++ b/pkg/operator/controllers/muo/staticresources/deployment.yaml.tmpl @@ -36,10 +36,12 @@ spec: path: tls-ca-bundle.pem name: trusted-ca-bundle name: trusted-ca-bundle + {{ if .SupportsPodSecurityAdmission }} securityContext: runAsNonRoot: true seccompProfile: type: RuntimeDefault + {{ end }} containers: - name: managed-upgrade-operator # Replace this with the built image name @@ -74,9 +76,11 @@ spec: - mountPath: /etc/pki/ca-trust/extracted/pem name: trusted-ca-bundle readOnly: true + {{ if .SupportsPodSecurityAdmission }} securityContext: allowPrivilegeEscalation: false capabilities: drop: - ALL runAsNonRoot: true + {{ end }} diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go index f41294e2780..ab18e2505a4 100644 --- a/pkg/operator/deploy/deploy.go +++ b/pkg/operator/deploy/deploy.go @@ -28,6 +28,7 @@ import ( appsv1client "k8s.io/client-go/kubernetes/typed/apps/v1" corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "k8s.io/client-go/util/retry" + "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "github.com/Azure/ARO-RP/pkg/api" @@ -63,12 +64,13 @@ type operator struct { oc *api.OpenShiftCluster arocli aroclient.Interface + client client.Client extensionscli extensionsclient.Interface kubernetescli kubernetes.Interface dh dynamichelper.Interface } -func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli aroclient.Interface, extensionscli extensionsclient.Interface, kubernetescli kubernetes.Interface) (Operator, error) { +func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli aroclient.Interface, client client.Client, extensionscli extensionsclient.Interface, kubernetescli kubernetes.Interface) (Operator, error) { restConfig, err := restconfig.RestConfig(env, oc) if err != nil { return nil, err @@ -84,6 +86,7 @@ func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli oc: oc, arocli: arocli, + client: client, extensionscli: extensionscli, kubernetescli: kubernetescli, dh: dh, @@ -91,9 +94,10 @@ func New(log *logrus.Entry, env env.Interface, oc *api.OpenShiftCluster, arocli } type deploymentData struct { - Image string - Version string - IsLocalDevelopment bool + Image string + Version string + IsLocalDevelopment bool + SupportsPodSecurityAdmission bool } func templateManifests(data deploymentData) ([][]byte, error) { @@ -125,7 +129,7 @@ func templateManifests(data deploymentData) ([][]byte, error) { return templatedFiles, nil } -func (o *operator) createDeploymentData() deploymentData { +func (o *operator) createDeploymentData(ctx context.Context) (deploymentData, error) { image := o.env.AROOperatorImage() // HACK: Override for ARO_IMAGE env variable setup in local-dev mode @@ -141,15 +145,27 @@ func (o *operator) createDeploymentData() deploymentData { image = fmt.Sprintf("%s/aro:%s", o.env.ACRDomain(), version) } - return deploymentData{ - IsLocalDevelopment: o.env.IsLocalDevelopmentMode(), - Image: image, - Version: version, + // Set Pod Security Admission parameters if > 4.10 + // this only gets set on PUCM (Everything or OperatorUpdate) + usePodSecurityAdmission, err := pkgoperator.ShouldUsePodSecurityStandard(ctx, o.client) + if err != nil { + return deploymentData{}, err } + + return deploymentData{ + IsLocalDevelopment: o.env.IsLocalDevelopmentMode(), + Image: image, + SupportsPodSecurityAdmission: usePodSecurityAdmission, + Version: version, + }, nil } -func (o *operator) createObjects() ([]kruntime.Object, error) { - deploymentData := o.createDeploymentData() +func (o *operator) createObjects(ctx context.Context) ([]kruntime.Object, error) { + deploymentData, err := o.createDeploymentData(ctx) + if err != nil { + return nil, err + } + templated, err := templateManifests(deploymentData) if err != nil { return nil, err @@ -166,10 +182,10 @@ func (o *operator) createObjects() ([]kruntime.Object, error) { return objects, nil } -func (o *operator) resources() ([]kruntime.Object, error) { +func (o *operator) resources(ctx context.Context) ([]kruntime.Object, error) { // first static resources from Assets - results, err := o.createObjects() + results, err := o.createObjects(ctx) if err != nil { return nil, err } @@ -281,7 +297,7 @@ func (o *operator) resources() ([]kruntime.Object, error) { } func (o *operator) CreateOrUpdate(ctx context.Context) error { - resources, err := o.resources() + resources, err := o.resources(ctx) if err != nil { return err } diff --git a/pkg/operator/deploy/deploy_test.go b/pkg/operator/deploy/deploy_test.go index afe1bf5c18d..6002e65dd7d 100644 --- a/pkg/operator/deploy/deploy_test.go +++ b/pkg/operator/deploy/deploy_test.go @@ -9,10 +9,12 @@ import ( "testing" "github.com/golang/mock/gomock" + configv1 "github.com/openshift/api/config/v1" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/client-go/kubernetes/fake" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" "github.com/Azure/ARO-RP/pkg/api" "github.com/Azure/ARO-RP/pkg/util/cmp" @@ -136,7 +138,9 @@ func TestCreateDeploymentData(t *testing.T) { name string mock func(*mock_env.MockInterface, *api.OpenShiftCluster) operatorVersionOverride string + clusterVersion string expected deploymentData + wantErr string }{ { name: "no image override, use default", @@ -145,6 +149,7 @@ func TestCreateDeploymentData(t *testing.T) { AROOperatorImage(). Return(operatorImageWithTag) }, + clusterVersion: "4.10.0", expected: deploymentData{ Image: operatorImageWithTag, Version: operatorImageTag, @@ -157,6 +162,7 @@ func TestCreateDeploymentData(t *testing.T) { AROOperatorImage(). Return(operatorImageUntagged) }, + clusterVersion: "4.10.0", expected: deploymentData{ Image: operatorImageUntagged, Version: "latest", @@ -174,13 +180,29 @@ func TestCreateDeploymentData(t *testing.T) { oc.Properties.OperatorVersion = "override" }, + clusterVersion: "4.10.0", expected: deploymentData{ Image: "docker.io/aro:override", Version: "override", }, }, + { + name: "version supports pod security admission", + mock: func(env *mock_env.MockInterface, oc *api.OpenShiftCluster) { + env.EXPECT(). + AROOperatorImage(). + Return(operatorImageWithTag) + }, + clusterVersion: "4.11.0", + expected: deploymentData{ + Image: operatorImageWithTag, + Version: operatorImageTag, + SupportsPodSecurityAdmission: true, + }, + }, } { t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() controller := gomock.NewController(t) defer controller.Finish() @@ -190,12 +212,29 @@ func TestCreateDeploymentData(t *testing.T) { oc := &api.OpenShiftCluster{Properties: api.OpenShiftClusterProperties{}} tt.mock(env, oc) + cv := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{}, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + { + State: configv1.CompletedUpdate, + Version: tt.clusterVersion, + }, + }, + }, + } + o := operator{ - oc: oc, - env: env, + oc: oc, + env: env, + client: ctrlfake.NewClientBuilder().WithObjects(cv).Build(), } - deploymentData := o.createDeploymentData() + deploymentData, err := o.createDeploymentData(ctx) + utilerror.AssertErrorMessage(t, err, tt.wantErr) if !reflect.DeepEqual(deploymentData, tt.expected) { t.Errorf("actual deployment: %v, expected %v", deploymentData, tt.expected) } @@ -205,15 +244,17 @@ func TestCreateDeploymentData(t *testing.T) { func TestOperatorVersion(t *testing.T) { type test struct { - name string - oc func() *api.OpenShiftClusterProperties - wantVersion string - wantPullspec string + name string + clusterVersion string + oc func() *api.OpenShiftClusterProperties + wantVersion string + wantPullspec string } for _, tt := range []*test{ { - name: "default", + name: "default", + clusterVersion: "4.10.0", oc: func() *api.OpenShiftClusterProperties { return &api.OpenShiftClusterProperties{} }, @@ -221,7 +262,8 @@ func TestOperatorVersion(t *testing.T) { wantPullspec: "defaultaroimagefromenv", }, { - name: "overridden", + name: "overridden", + clusterVersion: "4.10.0", oc: func() *api.OpenShiftClusterProperties { return &api.OpenShiftClusterProperties{ OperatorVersion: "v20220101.0", @@ -232,6 +274,7 @@ func TestOperatorVersion(t *testing.T) { }, } { t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() oc := tt.oc() controller := gomock.NewController(t) @@ -242,12 +285,28 @@ func TestOperatorVersion(t *testing.T) { _env.EXPECT().AROOperatorImage().AnyTimes().Return("defaultaroimagefromenv") _env.EXPECT().IsLocalDevelopmentMode().AnyTimes().Return(false) + cv := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Spec: configv1.ClusterVersionSpec{}, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + { + State: configv1.CompletedUpdate, + Version: tt.clusterVersion, + }, + }, + }, + } + o := &operator{ - oc: &api.OpenShiftCluster{Properties: *oc}, - env: _env, + oc: &api.OpenShiftCluster{Properties: *oc}, + env: _env, + client: ctrlfake.NewClientBuilder().WithObjects(cv).Build(), } - staticResources, err := o.createObjects() + staticResources, err := o.createObjects(ctx) if err != nil { t.Error(err) } diff --git a/pkg/operator/deploy/staticresources/master/deployment.yaml.tmpl b/pkg/operator/deploy/staticresources/master/deployment.yaml.tmpl index 173133f8e24..a425c35f422 100644 --- a/pkg/operator/deploy/staticresources/master/deployment.yaml.tmpl +++ b/pkg/operator/deploy/staticresources/master/deployment.yaml.tmpl @@ -55,18 +55,22 @@ spec: httpGet: path: /healthz/ready port: 8080 + {{ if .SupportsPodSecurityAdmission }} securityContext: allowPrivilegeEscalation: false capabilities: drop: - ALL runAsNonRoot: true + {{ end }} nodeSelector: node-role.kubernetes.io/master: "" + {{ if .SupportsPodSecurityAdmission }} securityContext: runAsNonRoot: true seccompProfile: type: RuntimeDefault + {{ end }} serviceAccountName: aro-operator-master serviceAccount: aro-operator-master priorityClassName: system-cluster-critical diff --git a/pkg/operator/deploy/staticresources/worker/deployment.yaml.tmpl b/pkg/operator/deploy/staticresources/worker/deployment.yaml.tmpl index 3491bbce0f6..697cb34b17b 100644 --- a/pkg/operator/deploy/staticresources/worker/deployment.yaml.tmpl +++ b/pkg/operator/deploy/staticresources/worker/deployment.yaml.tmpl @@ -37,18 +37,22 @@ spec: httpGet: path: /healthz/ready port: 8080 + {{ if .SupportsPodSecurityAdmission }} securityContext: allowPrivilegeEscalation: false capabilities: drop: - ALL runAsNonRoot: true + {{ end }} nodeSelector: node-role.kubernetes.io/worker: "" + {{ if .SupportsPodSecurityAdmission }} securityContext: runAsNonRoot: true seccompProfile: type: RuntimeDefault + {{ end }} serviceAccountName: aro-operator-worker serviceAccount: aro-operator-worker priorityClassName: system-cluster-critical diff --git a/pkg/operator/helpers.go b/pkg/operator/helpers.go index f8f89dced42..0f4c223b208 100644 --- a/pkg/operator/helpers.go +++ b/pkg/operator/helpers.go @@ -4,9 +4,41 @@ package operator // Licensed under the Apache License 2.0. import ( + "context" + + configv1 "github.com/openshift/api/config/v1" + "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" + arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" + "github.com/Azure/ARO-RP/pkg/util/version" ) +var clusterVersionForPodSecurityStandard = version.NewVersion(4, 11) + func GatewayEnabled(cluster *arov1alpha1.Cluster) bool { return len(cluster.Spec.GatewayDomains) > 0 } + +// ShouldUsePodSecurityStandard is an admissions controller +// for pods which replaces pod security policies, enabled on +// OpenShift 4.11 and up +func ShouldUsePodSecurityStandard(ctx context.Context, client client.Client) (bool, error) { + cv := &configv1.ClusterVersion{} + + err := client.Get(ctx, types.NamespacedName{Name: "version"}, cv) + if err != nil { + return false, err + } + + vers, err := version.GetClusterVersion(cv) + if err != nil { + return false, err + } + + if vers.Lt(clusterVersionForPodSecurityStandard) { + return false, nil + } + + return true, nil +} diff --git a/pkg/operator/helpers_test.go b/pkg/operator/helpers_test.go index f26e695ed05..3ba62c7b792 100644 --- a/pkg/operator/helpers_test.go +++ b/pkg/operator/helpers_test.go @@ -4,12 +4,76 @@ package operator // Licensed under the Apache License 2.0. import ( + "context" "testing" + configv1 "github.com/openshift/api/config/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + ctrlfake "sigs.k8s.io/controller-runtime/pkg/client/fake" + arov1alpha1 "github.com/Azure/ARO-RP/pkg/operator/apis/aro.openshift.io/v1alpha1" _ "github.com/Azure/ARO-RP/pkg/util/scheme" + utilerror "github.com/Azure/ARO-RP/test/util/error" ) +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 TestPodSecurityAdmissionControl(t *testing.T) { + tests := []struct { + name string + cv configv1.ClusterVersion + wantPodSecurity bool + wantErr string + }{ + { + name: "cluster < 4.11, don't use pod security", + cv: clusterVersion("4.10.99"), + wantPodSecurity: false, + }, + { + name: "cluster >= 4.11, use pod security", + cv: clusterVersion("4.11.0"), + wantPodSecurity: true, + }, + { + 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() + client := ctrlfake.NewClientBuilder().WithObjects(&tt.cv).Build() + + gotUsePodSecurity, err := ShouldUsePodSecurityStandard(ctx, client) + utilerror.AssertErrorMessage(t, err, tt.wantErr) + + if gotUsePodSecurity != tt.wantPodSecurity { + t.Errorf("got: %v\nwanted:%v\n", gotUsePodSecurity, tt.wantPodSecurity) + } + } +} + func aroCluster(domains []string) *arov1alpha1.Cluster { return &arov1alpha1.Cluster{ Spec: arov1alpha1.ClusterSpec{ diff --git a/pkg/util/version/clusterversion.go b/pkg/util/version/clusterversion.go index 851ec06bb0d..370f294c2f8 100644 --- a/pkg/util/version/clusterversion.go +++ b/pkg/util/version/clusterversion.go @@ -9,12 +9,28 @@ import ( configv1 "github.com/openshift/api/config/v1" ) +// GetClusterVersion fetches the version of the openshift cluster. +// Note that it assumes the most recently applied version is +// cv.Status.History[0] assuming the State == Completed. +// If for some reason there is no cluster version history, it will +// return the most recently updated version in history func GetClusterVersion(cv *configv1.ClusterVersion) (*Version, error) { + unknownErr := errors.New("unknown cluster version") + if cv == nil { + return nil, unknownErr + } + for _, history := range cv.Status.History { if history.State == configv1.CompletedUpdate { return ParseVersion(history.Version) } } - return nil, errors.New("unknown cluster version") + // If the cluster history has no completed version, we're most likely installing + // so grab the first history version and use it even if it's not completed + if len(cv.Status.History) > 0 { + return ParseVersion(cv.Status.History[0].Version) + } + + return nil, unknownErr } diff --git a/pkg/util/version/clusterversion_test.go b/pkg/util/version/clusterversion_test.go new file mode 100644 index 00000000000..f3c8d0a4fe6 --- /dev/null +++ b/pkg/util/version/clusterversion_test.go @@ -0,0 +1,112 @@ +package version + +// Copyright (c) Microsoft Corporation. +// Licensed under the Apache License 2.0. + +import ( + "reflect" + "testing" + + configv1 "github.com/openshift/api/config/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + + utilerror "github.com/Azure/ARO-RP/test/util/error" +) + +func TestGetClusterVersion(t *testing.T) { + for _, tt := range []struct { + name string + wantVers *Version + mocks func(*configv1.ClusterVersion) + wantErr string + }{ + { + name: "cluster version nil returns error", + //nolint:staticcheck // Ignore: SA4009 argument cv is overwritten before first use (staticcheck) + mocks: func(cv *configv1.ClusterVersion) { + cv = nil + }, + wantErr: "unknown cluster version", + }, + { + name: "no update history returns error", + wantErr: "unknown cluster version", + }, + { + name: "multiple completed updates returns top most", + wantVers: NewVersion(4, 10, 0), + mocks: func(cv *configv1.ClusterVersion) { + cv.Status.History = append(cv.Status.History, + configv1.UpdateHistory{ + State: configv1.CompletedUpdate, + Version: "4.10.0", + }, + configv1.UpdateHistory{ + State: configv1.CompletedUpdate, + Version: "4.9.0", + }, + ) + }, + }, + { + name: "pending update topmost, returns most recent completed update", + wantVers: NewVersion(4, 9, 0), + mocks: func(cv *configv1.ClusterVersion) { + cv.Status.History = append(cv.Status.History, + configv1.UpdateHistory{ + State: configv1.PartialUpdate, + Version: "4.10.0", + }, + configv1.UpdateHistory{ + State: configv1.CompletedUpdate, + Version: "4.9.0", + }, + ) + }, + }, + { + name: "only partial update in history returns partial update", + wantVers: NewVersion(4, 10, 0), + mocks: func(cv *configv1.ClusterVersion) { + cv.Status.History = append(cv.Status.History, + configv1.UpdateHistory{ + State: configv1.PartialUpdate, + Version: "4.10.0", + }, + ) + }, + }, + { + name: "missing update history state and no completed returns version", + wantVers: NewVersion(4, 10, 0), + mocks: func(cv *configv1.ClusterVersion) { + cv.Status.History = append(cv.Status.History, + configv1.UpdateHistory{ + Version: "4.10.0", + }, + ) + }, + }, + } { + t.Run(tt.name, func(t *testing.T) { + cv := &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "version", + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{}, + }, + } + + if tt.mocks != nil { + tt.mocks(cv) + } + version, err := GetClusterVersion(cv) + utilerror.AssertErrorMessage(t, err, tt.wantErr) + + if !reflect.DeepEqual(version, tt.wantVers) { + t.Error(version) + } + }) + } +}