From 936661226bf92528d3a49da4189365c70609f5a5 Mon Sep 17 00:00:00 2001 From: Jiri Podivin Date: Thu, 11 Apr 2024 11:36:37 +0200 Subject: [PATCH] Introducing spec level validation of dataplane/controlplane TLS consistency Verifies that TLS settings for nodeset are consistent with those of existing control plane, if there is one and only one. If there are multiple control planes the process will result in error, same if it isn't possible to retrieve list of control planes. Tests are included Signed-off-by: Jiri Podivin --- .../openstackdataplanenodeset_types.go | 47 +++++ config/rbac/role.yaml | 8 + ...openstackdataplanedeployment_controller.go | 12 ++ go.mod | 2 +- tests/functional/base_test.go | 33 +++ ...tackdataplanedeployment_controller_test.go | 104 ++++++++++ tests/functional/suite_test.go | 2 +- .../00-controlplane-create.yaml | 190 ++++++++++++++++++ .../00-dataplane-create.yaml | 2 +- 9 files changed, 397 insertions(+), 3 deletions(-) create mode 100644 tests/kuttl/tests/dataplane-deploy-global-service-test/00-controlplane-create.yaml diff --git a/api/v1beta1/openstackdataplanenodeset_types.go b/api/v1beta1/openstackdataplanenodeset_types.go index 83e3a9bb5..ebb5ab35c 100644 --- a/api/v1beta1/openstackdataplanenodeset_types.go +++ b/api/v1beta1/openstackdataplanenodeset_types.go @@ -17,9 +17,11 @@ limitations under the License. package v1beta1 import ( + "context" "fmt" "golang.org/x/exp/slices" + "sigs.k8s.io/controller-runtime/pkg/client" infranetworkv1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1" condition "github.com/openstack-k8s-operators/lib-common/modules/common/condition" @@ -290,3 +292,48 @@ func (r *OpenStackDataPlaneNodeSetSpec) duplicateNodeCheck(nodeSetList *OpenStac return } + +// Compare TLS settings of control plane and data plane +// if control plane name is specified attempt to retrieve it +// otherwise get any control plane in the namespace +func (r *OpenStackDataPlaneNodeSetSpec) ValidateTLS(namespace string, reconcilerClient client.Client, ctx context.Context) error { + var err error + + controlPlanes := openstackv1.OpenStackControlPlaneList{} + opts := client.ListOptions{ + Namespace: namespace, + } + + // Attempt to get list of all ControlPlanes fail if that isn't possible + if err = reconcilerClient.List(ctx, &controlPlanes, &opts); err != nil { + return err + } + + // Verify TLS status of control plane only if there is a single one + // report error if there are multiple, or proceed if there are none + if len(controlPlanes.Items) > 1 { + err = fmt.Errorf("multiple control planes found in the namespace %v", namespace) + } else if len(controlPlanes.Items) == 1 { + controlPlane := controlPlanes.Items[0] + err = r.TLSMatch(controlPlane) + } + + return err +} + +// Do TLS flags match in control plane ingress, pods and data plane +func (r *OpenStackDataPlaneNodeSetSpec) TLSMatch(controlPlane openstackv1.OpenStackControlPlane) *field.Error { + + if controlPlane.Spec.TLS.Ingress.Enabled != r.TLSEnabled || controlPlane.Spec.TLS.PodLevel.Enabled != r.TLSEnabled { + + return field.Forbidden( + field.NewPath("spec.tlsEnabled"), + fmt.Sprintf( + "TLS settings on Data Plane node set and Control Plane %s do not match, Node set: %t Control Plane Ingress: %t Control Plane PodLevel: %t", + controlPlane.Name, + r.TLSEnabled, + controlPlane.Spec.TLS.Ingress.Enabled, + controlPlane.Spec.TLS.PodLevel.Enabled)) + } + return nil +} diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index fdfd16163..a7957350a 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -185,6 +185,14 @@ rules: - patch - update - watch +- apiGroups: + - core.openstack.org + resources: + - openstackcontrolplanes + verbs: + - get + - list + - watch - apiGroups: - core.openstack.org resources: diff --git a/controllers/openstackdataplanedeployment_controller.go b/controllers/openstackdataplanedeployment_controller.go index 50ae20bac..7c0cb7936 100644 --- a/controllers/openstackdataplanedeployment_controller.go +++ b/controllers/openstackdataplanedeployment_controller.go @@ -62,6 +62,7 @@ func (r *OpenStackDataPlaneDeploymentReconciler) GetLogger(ctx context.Context) //+kubebuilder:rbac:groups=discovery.k8s.io,resources=endpointslices,verbs=get;list;watch;create;update;patch;delete; //+kubebuilder:rbac:groups=cert-manager.io,resources=issuers,verbs=get;list;watch; //+kubebuilder:rbac:groups=cert-manager.io,resources=certificates,verbs=get;list;watch;create;update;patch;delete; +//+kubebuilder:rbac:groups=core.openstack.org,resources=openstackcontrolplanes,verbs=get;list;watch; // Reconcile is part of the main kubernetes reconciliation loop which aims to // move the current state of the cluster closer to the desired state. @@ -183,6 +184,17 @@ func (r *OpenStackDataPlaneDeploymentReconciler) Reconcile(ctx context.Context, // Error reading the object - requeue the request. return ctrl.Result{}, err } + err = nodeSetInstance.Spec.ValidateTLS(instance.GetNamespace(), r.Client, ctx) + if err != nil { + Log.Info("error while comparing TLS settings of nodeset %s with control plane: %w", nodeSet, err) + instance.Status.Conditions.MarkFalse( + dataplanev1.SetupReadyCondition, + condition.ErrorReason, + condition.SeverityError, + dataplanev1.DataPlaneNodeSetErrorMessage, + err.Error()) + return ctrl.Result{}, err + } nodeSets.Items = append(nodeSets.Items, *nodeSetInstance) } diff --git a/go.mod b/go.mod index 617ef47d5..52f469420 100644 --- a/go.mod +++ b/go.mod @@ -28,6 +28,7 @@ require ( k8s.io/api v0.28.10 k8s.io/apimachinery v0.28.10 k8s.io/client-go v0.28.10 + k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 sigs.k8s.io/controller-runtime v0.16.6 ) @@ -113,7 +114,6 @@ require ( k8s.io/component-base v0.28.10 // indirect k8s.io/klog/v2 v2.120.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect - k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 // indirect sigs.k8s.io/gateway-api v0.8.0 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect diff --git a/tests/functional/base_test.go b/tests/functional/base_test.go index 57996c4b1..c9450e1ee 100644 --- a/tests/functional/base_test.go +++ b/tests/functional/base_test.go @@ -9,6 +9,7 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/types" + "sigs.k8s.io/controller-runtime/pkg/client" dataplanev1 "github.com/openstack-k8s-operators/dataplane-operator/api/v1beta1" infrav1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1" @@ -344,6 +345,38 @@ func DefaultDataplaneGlobalService(name types.NamespacedName) map[string]interfa } } +// Create simple OpenStackControlPlane +func CreateOpenStackControlPlane(name types.NamespacedName, tlsEnabled bool) client.Object { + + raw := map[string]interface{}{ + "apiVersion": "core.openstack.org/v1beta1", + "kind": "OpenStackControlPlane", + "metadata": map[string]interface{}{ + "name": name.Name, + "namespace": name.Namespace, + }, + "spec": map[string]interface{}{ + "secret": "osp-secret", + "storageClass": "local-storage", + "tls": map[string]interface{}{ + "ingress": map[string]interface{}{ + "enabled": tlsEnabled, + "ca": map[string]interface{}{ + "duration": "100h", + }, + "cert": map[string]interface{}{ + "duration": "10h", + }, + }, + "podLevel": map[string]interface{}{ + "enabled": tlsEnabled, + }, + }, + }, + } + return th.CreateUnstructured(raw) +} + // Get resources // Retrieve OpenStackDataPlaneDeployment and check for errors diff --git a/tests/functional/openstackdataplanedeployment_controller_test.go b/tests/functional/openstackdataplanedeployment_controller_test.go index 3cd18e2b3..c268caae9 100644 --- a/tests/functional/openstackdataplanedeployment_controller_test.go +++ b/tests/functional/openstackdataplanedeployment_controller_test.go @@ -16,6 +16,7 @@ import ( baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/utils/ptr" "k8s.io/apimachinery/pkg/types" ) @@ -36,6 +37,7 @@ var _ = Describe("Dataplane Deployment Test", func() { var dataplaneServiceName types.NamespacedName var dataplaneUpdateServiceName types.NamespacedName var dataplaneGlobalServiceName types.NamespacedName + var controlPlaneName types.NamespacedName BeforeEach(func() { dnsMasqName = types.NamespacedName{ @@ -719,4 +721,106 @@ var _ = Describe("Dataplane Deployment Test", func() { ) }) }) + + When("A user sets TLSEnabled to true with control plane TLS disabled", func() { + BeforeEach(func() { + controlPlaneName = types.NamespacedName{ + Name: "mock-control-plane", + Namespace: namespace, + } + CreateSSHSecret(dataplaneSSHSecretName) + DeferCleanup(th.DeleteInstance, th.CreateSecret(neutronOvnMetadataSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(novaNeutronMetadataSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(novaCellComputeConfigSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(novaMigrationSSHKey, map[string][]byte{ + "ssh-privatekey": []byte("fake-ssh-private-key"), + "ssh-publickey": []byte("fake-ssh-public-key"), + })) + DeferCleanup(th.DeleteInstance, th.CreateSecret(ceilometerConfigSecretName, map[string][]byte{ + "fake_keys": []byte("blih"), + })) + // DefaultDataPlanenodeSetSpec comes with two mock services, one marked for deployment on all nodesets + CreateDataplaneService(dataplaneServiceName, false) + CreateDataplaneService(dataplaneGlobalServiceName, true) + + DeferCleanup(th.DeleteService, dataplaneServiceName) + DeferCleanup(th.DeleteService, dataplaneGlobalServiceName) + DeferCleanup(th.DeleteInstance, CreateNetConfig(dataplaneNetConfigName, DefaultNetConfigSpec())) + DeferCleanup(th.DeleteInstance, CreateDNSMasq(dnsMasqName, DefaultDNSMasqSpec())) + SimulateDNSMasqComplete(dnsMasqName) + DeferCleanup(th.DeleteInstance, CreateDataplaneNodeSet(dataplaneNodeSetName, DefaultDataPlaneNodeSetSpec(dataplaneNodeSetName.Name))) + DeferCleanup(th.DeleteInstance, CreateDataplaneDeployment(dataplaneDeploymentName, DefaultDataPlaneDeploymentSpec())) + SimulateIPSetComplete(dataplaneNodeName) + SimulateDNSDataComplete(dataplaneNodeSetName) + + DeferCleanup(th.DeleteInstance, CreateOpenStackControlPlane(controlPlaneName, false)) + }) + + It("Should have Spec fields initialized", func() { + dataplaneDeploymentInstance := GetDataplaneDeployment(dataplaneDeploymentName) + expectedSpec := dataplanev1.OpenStackDataPlaneDeploymentSpec{ + NodeSets: []string{"edpm-compute-nodeset"}, + AnsibleTags: "", + AnsibleLimit: "", + AnsibleSkipTags: "", + DeploymentRequeueTime: 15, + ServicesOverride: nil, + BackoffLimit: ptr.To(int32(6)), + } + Expect(dataplaneDeploymentInstance.Spec).Should(Equal(expectedSpec)) + }) + + It("should have ready condiction set to false and input condition set to unknown", func() { + + nodeSet := dataplanev1.OpenStackDataPlaneNodeSet{} + baremetal := baremetalv1.OpenStackBaremetalSet{ + ObjectMeta: metav1.ObjectMeta{ + Name: nodeSet.Name, + Namespace: nodeSet.Namespace, + }, + } + // Create config map for OVN service + ovnConfigMapName := types.NamespacedName{ + Namespace: namespace, + Name: "ovncontroller-config", + } + mapData := map[string]interface{}{ + "ovsdb-config": "test-ovn-config", + } + th.CreateConfigMap(ovnConfigMapName, mapData) + + nodeSet = *GetDataplaneNodeSet(dataplaneNodeSetName) + + // Set baremetal provisioning conditions to True + Eventually(func(g Gomega) { + // OpenStackBaremetalSet has the same name as OpenStackDataPlaneNodeSet + g.Expect(th.K8sClient.Get(th.Ctx, dataplaneNodeSetName, &baremetal)).To(Succeed()) + baremetal.Status.Conditions.MarkTrue( + condition.ReadyCondition, + condition.ReadyMessage) + g.Expect(th.K8sClient.Status().Update(th.Ctx, &baremetal)).To(Succeed()) + + }, th.Timeout, th.Interval).Should(Succeed()) + + th.ExpectCondition( + dataplaneDeploymentName, + ConditionGetterFunc(DataplaneDeploymentConditionGetter), + condition.ReadyCondition, + corev1.ConditionFalse, + ) + th.ExpectCondition( + dataplaneDeploymentName, + ConditionGetterFunc(DataplaneDeploymentConditionGetter), + condition.InputReadyCondition, + corev1.ConditionUnknown, + ) + }) + + }) }) diff --git a/tests/functional/suite_test.go b/tests/functional/suite_test.go index ef62249b2..f6e45bc97 100644 --- a/tests/functional/suite_test.go +++ b/tests/functional/suite_test.go @@ -26,6 +26,7 @@ import ( "github.com/google/uuid" . "github.com/onsi/ginkgo/v2" //revive:disable:dot-imports . "github.com/onsi/gomega" //revive:disable:dot-imports + openstackv1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1" "k8s.io/client-go/kubernetes" "k8s.io/client-go/kubernetes/scheme" ctrl "sigs.k8s.io/controller-runtime" @@ -46,7 +47,6 @@ import ( infrav1 "github.com/openstack-k8s-operators/infra-operator/apis/network/v1beta1" aee "github.com/openstack-k8s-operators/openstack-ansibleee-operator/api/v1beta1" baremetalv1 "github.com/openstack-k8s-operators/openstack-baremetal-operator/api/v1beta1" - openstackv1 "github.com/openstack-k8s-operators/openstack-operator/apis/core/v1beta1" //revive:disable-next-line:dot-imports . "github.com/openstack-k8s-operators/lib-common/modules/common/test/helpers" diff --git a/tests/kuttl/tests/dataplane-deploy-global-service-test/00-controlplane-create.yaml b/tests/kuttl/tests/dataplane-deploy-global-service-test/00-controlplane-create.yaml new file mode 100644 index 000000000..da7b47f8f --- /dev/null +++ b/tests/kuttl/tests/dataplane-deploy-global-service-test/00-controlplane-create.yaml @@ -0,0 +1,190 @@ +apiVersion: core.openstack.org/v1beta1 +kind: OpenStackControlPlane +metadata: + name: openstack-basic +spec: + secret: osp-secret + storageClass: local-storage + keystone: + template: + databaseInstance: openstack + secret: osp-secret + galera: + templates: + openstack: + storageClass: local-storage + storageRequest: 500M + secret: osp-secret + replicas: 1 + openstack-cell1: + storageClass: local-storage + storageRequest: 500M + secret: osp-secret + replicas: 1 + rabbitmq: + templates: + rabbitmq: + replicas: 1 + rabbitmq-cell1: + replicas: 1 + memcached: + templates: + memcached: + replicas: 1 + barbican: + template: + databaseInstance: openstack + secret: osp-secret + barbicanAPI: + replicas: 1 + barbicanWorker: + replicas: 1 + barbicanKeystoneListener: + replicas: 1 + placement: + template: + databaseInstance: openstack + secret: osp-secret + glance: + template: + secret: osp-secret + databaseInstance: openstack + storageClass: "" + storageRequest: 10G + keystoneEndpoint: default + glanceAPIs: + default: + type: single + replicas: 1 + cinder: + template: + databaseInstance: openstack + secret: osp-secret + cinderAPI: + replicas: 1 + cinderScheduler: + replicas: 1 + cinderBackup: + replicas: 0 # backend needs to be configured + cinderVolumes: + volume1: + replicas: 0 # backend needs to be configured + manila: + template: + manilaAPI: + replicas: 1 + manilaScheduler: + replicas: 1 + manilaShares: + share1: + replicas: 1 + ovn: + template: + ovnDBCluster: + ovndbcluster-nb: + replicas: 1 + dbType: NB + storageRequest: 10G + ovndbcluster-sb: + replicas: 1 + dbType: SB + storageRequest: 10G + ovnNorthd: + replicas: 1 + ovnController: {} + neutron: + template: + databaseInstance: openstack + secret: osp-secret + horizon: + template: + replicas: 1 + secret: osp-secret + nova: + template: + secret: osp-secret + heat: + enabled: false + template: + databaseInstance: openstack + heatAPI: + replicas: 1 + heatEngine: + replicas: 1 + secret: osp-secret + ironic: + enabled: false + template: + databaseInstance: openstack + ironicAPI: + replicas: 1 + ironicConductors: + - replicas: 1 + storageRequest: 10G + ironicInspector: + replicas: 1 + ironicNeutronAgent: + replicas: 1 + secret: osp-secret + telemetry: + enabled: true + template: + metricStorage: + enabled: false + monitoringStack: + alertingEnabled: true + scrapeInterval: 30s + storage: + strategy: persistent + retention: 24h + persistent: + pvcStorageRequest: 20G + autoscaling: + enabled: false + aodh: + passwordSelectors: + databaseAccount: aodh + databaseInstance: openstack + secret: osp-secret + heatInstance: heat + ceilometer: + enabled: true + secret: osp-secret + logging: + enabled: false + network: internalapi + ipaddr: 172.17.0.80 + port: 10514 + cloNamespace: openshift-logging + swift: + enabled: true + template: + swiftRing: + ringReplicas: 1 + swiftStorage: + replicas: 1 + swiftProxy: + replicas: 1 + octavia: + enabled: false + template: + databaseInstance: openstack + octaviaAPI: + replicas: 1 + secret: osp-secret + designate: + template: + databaseInstance: openstack + secret: osp-secret + designateAPI: + replicas: 1 + designateCentral: + replicas: 0 # backend needs to be configured + designateWorker: + replicas: 0 # backend needs to be configured + designateProducer: + replicas: 0 # backend needs to be configured + designateMdns: + replicas: 0 # backend needs to be configured + designateBackendbind9: + replicas: 0 # backend needs to be configured diff --git a/tests/kuttl/tests/dataplane-deploy-global-service-test/00-dataplane-create.yaml b/tests/kuttl/tests/dataplane-deploy-global-service-test/00-dataplane-create.yaml index 9aff2f749..5a7e107c4 100644 --- a/tests/kuttl/tests/dataplane-deploy-global-service-test/00-dataplane-create.yaml +++ b/tests/kuttl/tests/dataplane-deploy-global-service-test/00-dataplane-create.yaml @@ -141,7 +141,7 @@ spec: memReqs: gbReq: {} preProvisioned: true - tlsEnabled: false + tlsEnabled: true services: - download-cache - bootstrap