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 <jpodivin@redhat.com>

Author: jpodivin

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
+}

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:

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)
 	}
 

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

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

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,
+			)
+		})
+
+	})
 })

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"

tests/kuttl/tests/dataplane-deploy-global-service-test/00-assert.yaml

@@ -33,7 +33,7 @@ spec:
   - libvirt
   - nova
   - custom-global-service
-  tlsEnabled: false
+  tlsEnabled: true
   env:
   - name: ANSIBLE_FORCE_COLOR
     value: "True"