Use safe RFC1123 for network policy names (#515)

Network policy names are based on the ServiceIdentity name they are created from. The service identity name might contain underscores, which are invalid characters for k8s objects (which must be valid DNS names according to RFC1123). Instead, use a name with the underscores replaced. The implementation of ServiceIdentity is not changed since that might have far-reaching effects in the system.
otterize · Nov 17, 2024 · 8d77de8 · 8d77de8
1 parent 46abcae
commit 8d77de8
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 6 deletions.
diff --git a/src/operator/controllers/intents_reconcilers/networkpolicy/reconciler.go b/src/operator/controllers/intents_reconcilers/networkpolicy/reconciler.go
@@ -187,10 +187,12 @@ func (r *Reconciler) applyServiceEffectivePolicy(ctx context.Context, ep effecti
 
 func (r *Reconciler) applyNetpol(ctx context.Context, ep effectivepolicy.ServiceEffectivePolicy, netpol v1.NetworkPolicy) error {
 	existingPolicy := &v1.NetworkPolicy{}
+
 	err := r.Get(ctx, types.NamespacedName{
 		Name:      netpol.Name,
 		Namespace: ep.Service.Namespace},
 		existingPolicy)
+
 	if err != nil && !k8serrors.IsNotFound(err) {
 		r.RecordWarningEventf(existingPolicy, consts.ReasonGettingNetworkPolicyFailed, "failed to get network policy: %s", err.Error())
 		return errors.Wrap(err)
@@ -353,9 +355,10 @@ func (r *Reconciler) buildSinglePolicy(ep effectivepolicy.ServiceEffectivePolicy
 	if shouldCreateEgress {
 		policyTypes = append(policyTypes, v1.PolicyTypeEgress)
 	}
+
 	return v1.NetworkPolicy{
 		ObjectMeta: metav1.ObjectMeta{
-			Name:      fmt.Sprintf(otterizev2alpha1.OtterizeSingleNetworkPolicyNameTemplate, ep.Service.GetNameWithKind()),
+			Name:      fmt.Sprintf(otterizev2alpha1.OtterizeSingleNetworkPolicyNameTemplate, ep.Service.GetRFC1123NameWithKind()),
 			Namespace: ep.Service.Namespace,
 			Labels: map[string]string{
 				otterizev2alpha1.OtterizeNetworkPolicy: ep.Service.GetFormattedOtterizeIdentityWithKind(),
@@ -375,7 +378,7 @@ func (r *Reconciler) buildSeparatePolicies(ep effectivepolicy.ServiceEffectivePo
 	if shouldCreateIngress {
 		ingressPolicy := v1.NetworkPolicy{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      fmt.Sprintf(otterizev2alpha1.OtterizeNetworkPolicyIngressNameTemplate, ep.Service.GetNameWithKind()),
+				Name:      fmt.Sprintf(otterizev2alpha1.OtterizeNetworkPolicyIngressNameTemplate, ep.Service.GetRFC1123NameWithKind()),
 				Namespace: ep.Service.Namespace,
 				Labels: map[string]string{
 					otterizev2alpha1.OtterizeNetworkPolicy: ep.Service.GetFormattedOtterizeIdentityWithKind(),
@@ -392,7 +395,7 @@ func (r *Reconciler) buildSeparatePolicies(ep effectivepolicy.ServiceEffectivePo
 	if shouldCreateEgress {
 		egressPolicy := v1.NetworkPolicy{
 			ObjectMeta: metav1.ObjectMeta{
-				Name:      fmt.Sprintf(otterizev2alpha1.OtterizeNetworkPolicyEgressNameTemplate, ep.Service.GetNameWithKind()),
+				Name:      fmt.Sprintf(otterizev2alpha1.OtterizeNetworkPolicyEgressNameTemplate, ep.Service.GetRFC1123NameWithKind()),
 				Namespace: ep.Service.Namespace,
 				Labels: map[string]string{
 					otterizev2alpha1.OtterizeNetworkPolicy: ep.Service.GetFormattedOtterizeIdentityWithKind(),

diff --git a/src/operator/controllers/istiopolicy/policy_manager.go b/src/operator/controllers/istiopolicy/policy_manager.go
@@ -458,9 +458,9 @@ func (c *PolicyManagerImpl) updatePolicy(ctx context.Context, existingPolicy *v1
 
 func (c *PolicyManagerImpl) getPolicyName(intents *v2alpha1.ClientIntents, intent v2alpha1.Target) string {
 	clientIdentity := intents.ToServiceIdentity()
-	clientName := fmt.Sprintf("%s.%s", clientIdentity.GetNameWithKind(), intents.Namespace)
+	clientName := fmt.Sprintf("%s.%s", clientIdentity.GetRFC1123NameWithKind(), intents.Namespace)
 	serverIdentity := intent.ToServiceIdentity(intents.Namespace)
-	policyName := fmt.Sprintf(OtterizeIstioPolicyNameTemplate, serverIdentity.GetNameWithKind(), clientName)
+	policyName := fmt.Sprintf(OtterizeIstioPolicyNameTemplate, serverIdentity.GetRFC1123NameWithKind(), clientName)
 	return policyName
 }
 

diff --git a/src/operator/controllers/protected_service_reconcilers/default_deny.go b/src/operator/controllers/protected_service_reconcilers/default_deny.go
@@ -141,7 +141,7 @@ func (r *DefaultDenyReconciler) updateIfNeeded(
 }
 
 func (r *DefaultDenyReconciler) buildNetworkPolicyObjectForIntent(ctx context.Context, serviceId serviceidentity.ServiceIdentity) (v1.NetworkPolicy, bool, error) {
-	policyName := fmt.Sprintf("default-deny-%s", serviceId.GetNameWithKind())
+	policyName := fmt.Sprintf("default-deny-%s", serviceId.GetRFC1123NameWithKind())
 	podSelectorLabels, ok, err := otterizev2alpha1.ServiceIdentityToLabelsForWorkloadSelection(ctx, r.Client, serviceId)
 	if err != nil {
 		return v1.NetworkPolicy{}, false, errors.Wrap(err)

diff --git a/src/shared/serviceidresolver/serviceidentity/serviceidentity.go b/src/shared/serviceidresolver/serviceidentity/serviceidentity.go
@@ -48,6 +48,10 @@ func (si ServiceIdentity) GetNameWithKind() string {
 	return lo.Ternary(si.Kind == "" || si.Kind == KindOtterizeLegacy, si.Name, fmt.Sprintf("%s-%s", si.Name, strings.ToLower(si.Kind)))
 }
 
+func (si ServiceIdentity) GetRFC1123NameWithKind() string {
+	return strings.ReplaceAll(si.GetNameWithKind(), "_", ".")
+}
+
 func (si ServiceIdentity) Equals(other ServiceIdentity) bool {
 	return si.Name == other.Name && si.Namespace == other.Namespace && si.Kind == other.Kind
 }