From 99c52b51eb273097ca07c67dfbf9d42db134ca67 Mon Sep 17 00:00:00 2001 From: Brendan Bergen Date: Thu, 17 Feb 2022 13:28:40 -0700 Subject: [PATCH] Re-enable Egress Lockdown Enable egress lockdown feature by default on new clusters while also allowing current clusters to be admin-upgraded with the new feature Co-authored-by: Ben Vesel <10840174+bennerv@users.noreply.github.com> --- .../admin/openshiftcluster_validatestatic.go | 4 -- .../openshiftcluster_validatestatic_test.go | 19 ++++--- pkg/frontend/openshiftcluster_putorpatch.go | 7 ++- .../openshiftcluster_putorpatch_test.go | 2 +- pkg/operator/deploy/deploy.go | 10 ++-- test/e2e/cluster.go | 49 +++++++++++++++++++ 6 files changed, 71 insertions(+), 20 deletions(-) diff --git a/pkg/api/admin/openshiftcluster_validatestatic.go b/pkg/api/admin/openshiftcluster_validatestatic.go index f94c6a338b3..bb56358e487 100644 --- a/pkg/api/admin/openshiftcluster_validatestatic.go +++ b/pkg/api/admin/openshiftcluster_validatestatic.go @@ -33,9 +33,5 @@ func (sv *openShiftClusterStaticValidator) validateDelta(oc, current *OpenShiftC return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodeInvalidParameter, "properties.maintenanceTask", "Invalid enum parameter.") } - if current.Properties.FeatureProfile.GatewayEnabled && !oc.Properties.FeatureProfile.GatewayEnabled { - return api.NewCloudError(http.StatusBadRequest, api.CloudErrorCodePropertyChangeNotAllowed, "properties.featureProfile.gatewayEnabled", "Changing property 'properties.featureProfile.gatewayEnabled' is not allowed.") - } - return nil } diff --git a/pkg/api/admin/openshiftcluster_validatestatic_test.go b/pkg/api/admin/openshiftcluster_validatestatic_test.go index a044486540f..a00ca327fe3 100644 --- a/pkg/api/admin/openshiftcluster_validatestatic_test.go +++ b/pkg/api/admin/openshiftcluster_validatestatic_test.go @@ -129,29 +129,34 @@ func TestOpenShiftClusterStaticValidateDelta(t *testing.T) { wantErr: "400: PropertyChangeNotAllowed: properties.lastAdminUpdateError: Changing property 'properties.lastAdminUpdateError' is not allowed.", }, { - name: "valid gatewayEnabled change", + name: "disable gatewayEnabled on enabled clusters is allowed", oc: func() *OpenShiftCluster { - return &OpenShiftCluster{} + return &OpenShiftCluster{ + Properties: OpenShiftClusterProperties{ + FeatureProfile: FeatureProfile{ + GatewayEnabled: true, + }, + }, + } }, modify: func(oc *OpenShiftCluster) { - oc.Properties.FeatureProfile.GatewayEnabled = true + oc.Properties.FeatureProfile.GatewayEnabled = false }, }, { - name: "valid gatewayEnabled change", + name: "enable gatewayEnabled on disabled clusters is allowed", oc: func() *OpenShiftCluster { return &OpenShiftCluster{ Properties: OpenShiftClusterProperties{ FeatureProfile: FeatureProfile{ - GatewayEnabled: true, + GatewayEnabled: false, }, }, } }, modify: func(oc *OpenShiftCluster) { - oc.Properties.FeatureProfile.GatewayEnabled = false + oc.Properties.FeatureProfile.GatewayEnabled = true }, - wantErr: "400: PropertyChangeNotAllowed: properties.featureProfile.gatewayEnabled: Changing property 'properties.featureProfile.gatewayEnabled' is not allowed.", }, { name: "console url change is not allowed", diff --git a/pkg/frontend/openshiftcluster_putorpatch.go b/pkg/frontend/openshiftcluster_putorpatch.go index 087770e6d80..19fece5b47b 100644 --- a/pkg/frontend/openshiftcluster_putorpatch.go +++ b/pkg/frontend/openshiftcluster_putorpatch.go @@ -84,10 +84,9 @@ func (f *frontend) _putOrPatchOpenShiftCluster(ctx context.Context, log *logrus. }, }, } - // TODO (BV): reenable gateway on create once we fix bugs - // if !f.env.IsLocalDevelopmentMode() /* not local dev or CI */ { - // doc.OpenShiftCluster.Properties.FeatureProfile.GatewayEnabled = true - // } + if !f.env.IsLocalDevelopmentMode() /* not local dev or CI */ { + doc.OpenShiftCluster.Properties.FeatureProfile.GatewayEnabled = true + } } doc.CorrelationData = correlationData diff --git a/pkg/frontend/openshiftcluster_putorpatch_test.go b/pkg/frontend/openshiftcluster_putorpatch_test.go index 6f18785df27..adeaefd23bf 100644 --- a/pkg/frontend/openshiftcluster_putorpatch_test.go +++ b/pkg/frontend/openshiftcluster_putorpatch_test.go @@ -719,7 +719,7 @@ func TestPutOrPatchOpenShiftCluster(t *testing.T) { EncryptionAtHost: api.EncryptionAtHostDisabled, }, FeatureProfile: api.FeatureProfile{ - GatewayEnabled: false, + GatewayEnabled: true, }, OperatorFlags: api.DefaultOperatorFlags.Copy(), }, diff --git a/pkg/operator/deploy/deploy.go b/pkg/operator/deploy/deploy.go index 7b08ba15dad..fae77da2c84 100644 --- a/pkg/operator/deploy/deploy.go +++ b/pkg/operator/deploy/deploy.go @@ -175,10 +175,12 @@ func (o *operator) resources() ([]kruntime.Object, error) { }, } - // TODO (BV): reenable gateway once we fix bugs - // if o.oc.Properties.NetworkProfile.GatewayPrivateEndpointIP != "" { - // cluster.Spec.GatewayDomains = append(o.env.GatewayDomains(), o.oc.Properties.ImageRegistryStorageAccountName+".blob."+o.env.Environment().StorageEndpointSuffix) - // } + if o.oc.Properties.FeatureProfile.GatewayEnabled && o.oc.Properties.NetworkProfile.GatewayPrivateEndpointIP != "" { + cluster.Spec.GatewayDomains = append(o.env.GatewayDomains(), o.oc.Properties.ImageRegistryStorageAccountName+".blob."+o.env.Environment().StorageEndpointSuffix) + } else { + // covers the case of an admin-disable, we need to update dnsmasq on each node + cluster.Spec.GatewayDomains = make([]string, 0) + } // create a secret here for genevalogging, later we will copy it to // the genevalogging namespace. diff --git a/test/e2e/cluster.go b/test/e2e/cluster.go index 2bcf0a9c0d2..1ea23478ec3 100644 --- a/test/e2e/cluster.go +++ b/test/e2e/cluster.go @@ -83,6 +83,18 @@ var _ = Describe("Cluster smoke test", func() { return ready.ServiceIsReady(svc) }, 5*time.Minute, 10*time.Second).Should(BeTrue()) }) + + // mainly we want to test the gateway/egress functionality - this request for the image will travel from + // node > gateway > storage account of the registry. + Specify("Can access and use the internal container registry", func() { + ctx := context.Background() + deployName := "internal-registry-deploy" + err := createContainerFromInternalContainerRegistryImage(ctx, clients.Kubernetes, deployName) + Expect(err).NotTo(HaveOccurred()) + + err = wait.PollImmediate(10*time.Second, 5*time.Minute, ready.CheckDeploymentIsReady(ctx, clients.Kubernetes.AppsV1().Deployments(testNamespace), deployName)) + Expect(err).NotTo(HaveOccurred()) + }) }) func createStatefulSet(ctx context.Context, cli kubernetes.Interface) error { @@ -167,3 +179,40 @@ func createLoadBalancerService(ctx context.Context, cli kubernetes.Interface, na _, err := cli.CoreV1().Services(testNamespace).Create(context.Background(), svc, metav1.CreateOptions{}) return err } + +func createContainerFromInternalContainerRegistryImage(ctx context.Context, cli kubernetes.Interface, name string) error { + deploy := &appsv1.Deployment{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: testNamespace, + }, + Spec: appsv1.DeploymentSpec{ + Replicas: to.Int32Ptr(1), + Selector: &metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": name, + }, + }, + Template: corev1.PodTemplateSpec{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{"app": name}, + }, + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "cli", + Image: "image-registry.openshift-image-registry.svc:5000/openshift/cli", + Command: []string{ + "/bin/sh", + "-c", + "while true; do sleep 1; done", + }, + }, + }, + }, + }, + }, + } + _, err := cli.AppsV1().Deployments(testNamespace).Create(context.Background(), deploy, metav1.CreateOptions{}) + return err +}