Merge pull request #898 from fao89/volnames

Validate dataplane volume names
openstack-k8s-operators · Jul 19, 2024 · d662542 · d662542
2 parents 27bade1 + 872ef1a
commit d662542
Show file tree

Hide file tree

Showing 8 changed files with 45 additions and 1,537 deletions.
diff --git a/apis/bases/dataplane.openstack.org_openstackdataplanenodesets.yaml b/apis/bases/dataplane.openstack.org_openstackdataplanenodesets.yaml
diff --git a/apis/dataplane/v1beta1/common.go b/apis/dataplane/v1beta1/common.go
@@ -102,10 +102,6 @@ type AnsibleOpts struct {
 
 // NodeSection defines the top level attributes inherited by nodes in the CR.
 type NodeSection struct {
-	// ExtraMounts containing files which can be mounted into an Ansible Execution Pod
-	// +kubebuilder:validation:Optional
-	ExtraMounts []storage.VolMounts `json:"extraMounts,omitempty"`
-
 	// Networks - Instance networks
 	// +kubebuilder:validation:Optional
 	Networks []infranetworkv1.IPSetNetwork `json:"networks,omitempty"`

diff --git a/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go b/apis/dataplane/v1beta1/openstackdataplanenodeset_webhook.go
@@ -28,6 +28,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
+	apimachineryvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -123,6 +124,18 @@ func (r *OpenStackDataPlaneNodeSet) ValidateCreate() (admission.Warnings, error)
 			r.Name,
 			fmt.Sprintf("Error validating OpenStackDataPlaneNodeSet name %s, name must follow RFC1123", r.Name)))
 	}
+	// Validate volume names
+	for _, emount := range r.Spec.NodeTemplate.ExtraMounts {
+		for _, vol := range emount.Volumes {
+			msgs := apimachineryvalidation.IsDNS1123Label(vol.Name)
+			for _, msg := range msgs {
+				errors = append(errors, field.Invalid(
+					field.NewPath("spec.nodeTemplate.extraMounts"),
+					vol.Name,
+					msg))
+			}
+		}
+	}
 	if len(errors) > 0 {
 		openstackdataplanenodesetlog.Info("validation failed", "name", r.Name)
 

diff --git a/apis/dataplane/v1beta1/zz_generated.deepcopy.go b/apis/dataplane/v1beta1/zz_generated.deepcopy.go
diff --git a/config/crd/bases/dataplane.openstack.org_openstackdataplanenodesets.yaml b/config/crd/bases/dataplane.openstack.org_openstackdataplanenodesets.yaml
diff --git a/pkg/dataplane/deployment.go b/pkg/dataplane/deployment.go
@@ -27,6 +27,7 @@ import (
 	slices "golang.org/x/exp/slices"
 	k8s_errors "k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/types"
+	apimachineryvalidation "k8s.io/apimachinery/pkg/util/validation"
 	ctrl "sigs.k8s.io/controller-runtime"
 
 	"github.com/iancoleman/strcase"
@@ -310,8 +311,12 @@ func (d *Deployer) addCertMounts(
 					}
 					projectedVolumeSource.Sources = append(projectedVolumeSource.Sources, volumeProjection)
 				}
+				volumeName := GetServiceCertsSecretName(d.NodeSet, service.Name, certKey, 0)
+				if len(volumeName) > apimachineryvalidation.DNS1123LabelMaxLength {
+					volumeName = volumeName[:apimachineryvalidation.DNS1123LabelMaxLength]
+				}
 				certVolume := corev1.Volume{
-					Name: GetServiceCertsSecretName(d.NodeSet, service.Name, certKey, 0),
+					Name: volumeName,
 					VolumeSource: corev1.VolumeSource{
 						Projected: &projectedVolumeSource,
 					},
@@ -323,7 +328,7 @@ func (d *Deployer) addCertMounts(
 				}
 
 				certVolumeMount := corev1.VolumeMount{
-					Name:      GetServiceCertsSecretName(d.NodeSet, service.Name, certKey, 0),
+					Name:      volumeName,
 					MountPath: path.Join(CertPaths, certMountDir, certKey),
 				}
 				volMounts.Volumes = append(volMounts.Volumes, certVolume)
@@ -341,8 +346,12 @@ func (d *Deployer) addCertMounts(
 			if err != nil {
 				return d.AeeSpec, err
 			}
+			volumeName := fmt.Sprintf("%s-%s", service.Name, service.Spec.CACerts)
+			if len(volumeName) > apimachineryvalidation.DNS1123LabelMaxLength {
+				volumeName = volumeName[:apimachineryvalidation.DNS1123LabelMaxLength]
+			}
 			cacertVolume := corev1.Volume{
-				Name: fmt.Sprintf("%s-%s", service.Name, service.Spec.CACerts),
+				Name: volumeName,
 				VolumeSource: corev1.VolumeSource{
 					Secret: &corev1.SecretVolumeSource{
 						SecretName: service.Spec.CACerts,
@@ -351,7 +360,7 @@ func (d *Deployer) addCertMounts(
 			}
 
 			cacertVolumeMount := corev1.VolumeMount{
-				Name:      fmt.Sprintf("%s-%s", service.Name, service.Spec.CACerts),
+				Name:      volumeName,
 				MountPath: path.Join(CACertPaths, service.Spec.EDPMServiceType),
 			}
 
@@ -401,9 +410,13 @@ func (d *Deployer) addServiceExtraMounts(
 		sort.Strings(keys)
 
 		for idx, key := range keys {
-			name := fmt.Sprintf("%s-%s", cm.Name, strconv.Itoa(idx))
+			volumeName := fmt.Sprintf("%s-%s", cm.Name, strconv.Itoa(idx))
+			if len(volumeName) > apimachineryvalidation.DNS1123LabelMaxLength {
+				limit := apimachineryvalidation.DNS1123LabelMaxLength - len(strconv.Itoa(idx))
+				volumeName = volumeName[:limit] + strconv.Itoa(idx)
+			}
 			volume := corev1.Volume{
-				Name: name,
+				Name: volumeName,
 				VolumeSource: corev1.VolumeSource{
 					ConfigMap: &corev1.ConfigMapVolumeSource{
 						LocalObjectReference: corev1.LocalObjectReference{
@@ -420,7 +433,7 @@ func (d *Deployer) addServiceExtraMounts(
 			}
 
 			volumeMount := corev1.VolumeMount{
-				Name:      name,
+				Name:      volumeName,
 				MountPath: path.Join(baseMountPath, key),
 				SubPath:   key,
 			}
@@ -443,9 +456,13 @@ func (d *Deployer) addServiceExtraMounts(
 		sort.Strings(keys)
 
 		for idx, key := range keys {
-			name := fmt.Sprintf("%s-%s", sec.Name, strconv.Itoa(idx))
+			volumeName := fmt.Sprintf("%s-%s", sec.Name, strconv.Itoa(idx))
+			if len(volumeName) > apimachineryvalidation.DNS1123LabelMaxLength {
+				limit := apimachineryvalidation.DNS1123LabelMaxLength - len(strconv.Itoa(idx))
+				volumeName = volumeName[:limit] + strconv.Itoa(idx)
+			}
 			volume := corev1.Volume{
-				Name: name,
+				Name: volumeName,
 				VolumeSource: corev1.VolumeSource{
 					Secret: &corev1.SecretVolumeSource{
 						SecretName: sec.Name,
@@ -460,7 +477,7 @@ func (d *Deployer) addServiceExtraMounts(
 			}
 
 			volumeMount := corev1.VolumeMount{
-				Name:      name,
+				Name:      volumeName,
 				MountPath: path.Join(baseMountPath, key),
 				SubPath:   key,
 			}

diff --git a/pkg/dataplane/util/ansible_execution.go b/pkg/dataplane/util/ansible_execution.go
@@ -27,6 +27,7 @@ import (
 	corev1 "k8s.io/api/core/v1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	apimachineryvalidation "k8s.io/apimachinery/pkg/util/validation"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
 
@@ -279,6 +280,7 @@ func GetAnsibleExecution(ctx context.Context,
 // getAnsibleExecutionNamePrefix compute the name of the AnsibleEE
 func getAnsibleExecutionNamePrefix(serviceName string) string {
 	var executionNamePrefix string
+	AnsibleExecutionServiceNameLen := apimachineryvalidation.DNS1123LabelMaxLength - 10
 	if len(serviceName) > AnsibleExecutionServiceNameLen {
 		executionNamePrefix = serviceName[:AnsibleExecutionServiceNameLen]
 	} else {
@@ -295,8 +297,9 @@ func GetAnsibleExecutionNameAndLabels(service *dataplanev1.OpenStackDataPlaneSer
 	if !service.Spec.DeployOnAllNodeSets {
 		executionName = fmt.Sprintf("%s-%s", executionName, nodeSetName)
 	}
-	if len(executionName) > AnsibleExcecutionNameLabelLen {
-		executionName = executionName[:AnsibleExcecutionNameLabelLen]
+
+	if len(executionName) > apimachineryvalidation.DNS1123LabelMaxLength {
+		executionName = executionName[:apimachineryvalidation.DNS1123LabelMaxLength]
 	}
 
 	labels := map[string]string{

diff --git a/pkg/dataplane/util/const.go b/pkg/dataplane/util/const.go