[TT-10958] Do not modify MID of Security Policy in OSS (#675)

* Do not modify MID of Security Policy in OSS Signed-off-by: Burak Sekili <[email protected]> * Check for ID in OSS instead of MID Signed-off-by: Burak Sekili <[email protected]> * Check tyk mode while validation status Signed-off-by: Burak Sekili <[email protected]> * Update changelog Signed-off-by: Burak Sekili <[email protected]> * Update tyk version matrix (#673) * Update run_tests.yaml * Update run_tests.yaml * Update run_tests.yaml * Update run_tests.yaml * Update run_tests.yaml --------- Signed-off-by: Burak Sekili <[email protected]> Co-authored-by: Komal Sukhani <[email protected]> Co-authored-by: Pranshu <[email protected]>
TykTechnologies · Jan 12, 2024 · 5a33c2e · 5a33c2e
1 parent e028ee5
commit 5a33c2e
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 36 deletions.
diff --git a/.github/workflows/run_tests.yaml b/.github/workflows/run_tests.yaml
@@ -39,7 +39,7 @@ jobs:
       fail-fast: false
       matrix: 
         k8s-version: ${{ fromJSON(needs.matrix-generator.outputs.k8s-matrix) }}
-        tyk-version: ['v3.2', 'v4.0', 'v4.3','v5.0.0']
+        tyk-version: ['v3.2','v4.0','v5.0','v5.2']
         mode: ['ce','pro']
 
     env:

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,10 @@
 ## [Unreleased](https://github.com/TykTechnologies/tyk-operator/tree/HEAD)
 [Full Changelog](https://github.com/TykTechnologies/tyk-operator/compare/v0.15.1...HEAD)
 
+**Fixed**:
+- Fixed a bug that prevents Tyk Operator to work with SecurityPolicy in CE (OSS) Mode.
+Now, SecurityPolicy controller will not modify `spec.MID` (`_id`) field in SecurityPolicy.
+
 **Added**:
 - Added `imagePullSecrets` configuration for ServiceAccount in Tyk Operator Helm chart 
 - Added `tyk` to `categories` field of CRDs. So, from now on, all CRs related to Tyk Operator is grouped

diff --git a/controllers/securitypolicy_controller.go b/controllers/securitypolicy_controller.go
@@ -246,11 +246,7 @@ func (r *SecurityPolicyReconciler) update(ctx context.Context,
 ) (*model.SecurityPolicySpec, error) {
 	r.Log.Info("Updating SecurityPolicy", "Policy ID", policy.Status.PolID)
 
-	if policy.Spec.MID == nil {
-		policy.Spec.MID = new(string)
-	}
-
-	*policy.Spec.MID = policy.Status.PolID
+	updatePolicyMID(ctx, &policy.Spec, &policy.Status.PolID)
 
 	spec, err := r.spec(ctx, &policy.Spec)
 	if err != nil {
@@ -288,12 +284,8 @@ func (r *SecurityPolicyReconciler) update(ctx context.Context,
 				return nil, err
 			}
 
-			if policy.Spec.MID == nil {
-				policy.Spec.MID = new(string)
-			}
-
-			*policy.Spec.MID = *spec.MID
-			policy.Status.PolID = *spec.MID
+			updatePolicyMID(ctx, &policy.Spec, spec.MID)
+			setPolicyStatusPolID(ctx, policy, spec)
 		} else {
 			r.Log.Error(err, "Failed to get Policy from Tyk", err)
 
@@ -315,7 +307,7 @@ func (r *SecurityPolicyReconciler) update(ctx context.Context,
 		return nil, err
 	}
 
-	polOnTyk, _ := klient.Universal.Portal().Policy().Get(ctx, *policy.Spec.MID) //nolint:errcheck
+	polOnTyk, _ := klient.Universal.Portal().Policy().Get(ctx, policy.Status.PolID) //nolint:errcheck
 
 	r.Log.Info("Successfully updated Policy")
 
@@ -325,6 +317,24 @@ func (r *SecurityPolicyReconciler) update(ctx context.Context,
 	})
 }
 
+func setPolicyStatusPolID(ctx context.Context, policy *tykv1.SecurityPolicy, spec *tykv1.SecurityPolicySpec) {
+	if policy == nil || spec == nil {
+		return
+	}
+
+	if env := opclient.GetTykMode(ctx); env.Mode == "ce" {
+		if spec.ID != nil {
+			policy.Status.PolID = *spec.ID
+		}
+
+		return
+	}
+
+	if spec.MID != nil {
+		policy.Status.PolID = *spec.MID
+	}
+}
+
 func (r *SecurityPolicyReconciler) create(ctx context.Context, policy *tykv1.SecurityPolicy) error {
 	r.Log.Info("Creating a policy")
 
@@ -351,11 +361,7 @@ func (r *SecurityPolicyReconciler) create(ctx context.Context, policy *tykv1.Sec
 			return err
 		}
 	} else {
-		if spec.MID == nil {
-			spec.MID = new(string)
-		}
-
-		*spec.MID = *existingSpec.MID
+		updatePolicyMID(ctx, spec, existingSpec.MID)
 
 		err = klient.Universal.Portal().Policy().Update(ctx, spec)
 		if err != nil {
@@ -380,11 +386,7 @@ func (r *SecurityPolicyReconciler) create(ctx context.Context, policy *tykv1.Sec
 
 	r.Log.Info("Successfully created Policy")
 
-	if policy.Spec.MID == nil {
-		policy.Spec.MID = new(string)
-	}
-
-	*policy.Spec.MID = *spec.MID
+	updatePolicyMID(ctx, &policy.Spec, spec.MID)
 
 	err = r.updateStatusOfLinkedAPIs(ctx, policy, false)
 	if err != nil {
@@ -396,14 +398,30 @@ func (r *SecurityPolicyReconciler) create(ctx context.Context, policy *tykv1.Sec
 		return err
 	}
 
-	polOnTyk, _ := klient.Universal.Portal().Policy().Get(ctx, *spec.MID) //nolint:errcheck
+	polOnTyk, _ := klient.Universal.Portal().Policy().Get(ctx, policy.Status.PolID) //nolint:errcheck
 
 	return r.updatePolicyStatus(ctx, policy, func(status *tykv1.SecurityPolicyStatus) {
 		status.LatestTykSpecHash = calculateHash(polOnTyk)
 		status.LatestCRDSpecHash = calculateHash(spec)
 	})
 }
 
+func updatePolicyMID(ctx context.Context, policy *tykv1.SecurityPolicySpec, mId *string) {
+	if env := opclient.GetTykMode(ctx); env.Mode == "ce" {
+		return
+	}
+
+	if mId == nil {
+		return
+	}
+
+	if policy.MID == nil {
+		policy.MID = new(string)
+	}
+
+	*policy.MID = *mId
+}
+
 // updatePolicyStatus updates the status of the policy.
 func (r *SecurityPolicyReconciler) updatePolicyStatus(
 	ctx context.Context,
@@ -412,9 +430,7 @@ func (r *SecurityPolicyReconciler) updatePolicyStatus(
 ) error {
 	r.Log.Info("Updating policy status")
 
-	if policy.Spec.MID != nil {
-		policy.Status.PolID = *policy.Spec.MID
-	}
+	setPolicyStatusPolID(ctx, policy, &policy.Spec)
 
 	if policy.Spec.AccessRightsArray != nil && len(policy.Spec.AccessRightsArray) > 0 {
 		policy.Status.LinkedAPIs = make([]model.Target, 0)

diff --git a/integration/securitypolicy_test.go b/integration/securitypolicy_test.go
@@ -321,7 +321,7 @@ func TestSecurityPolicyMigration(t *testing.T) {
 					tyk.State == initialK8sPolicyState
 			}
 
-			return *k8s.MID == *tyk.ID &&
+			return *k8s.ID == *tyk.ID &&
 				tyk.ID != nil &&
 				k8sID == *tyk.ID &&
 				len(k8s.Tags) == len(tyk.Tags) &&
@@ -620,7 +620,13 @@ func TestSecurityPolicy(t *testing.T) {
 							return false
 						}
 
-						eval.True(policyOnK8s.Status.PolID == *policyCR.Spec.MID)
+						if tykEnv.Mode == "ce" {
+							eval.True(policyCR.Spec.ID != nil)
+							eval.True(policyOnK8s.Status.PolID == *policyCR.Spec.ID)
+						} else {
+							eval.True(policyCR.Spec.MID != nil)
+							eval.True(policyOnK8s.Status.PolID == *policyCR.Spec.MID)
+						}
 						eval.Equal(policyOnK8s.Spec.Name, policyOnTyk.Name)
 						eval.Equal(len(policyOnK8s.Spec.AccessRightsArray), 1)
 

diff --git a/pkg/client/client.go b/pkg/client/client.go
@@ -100,6 +100,16 @@ func SetContext(ctx context.Context, rctx Context) context.Context {
 	return context.WithValue(ctx, contextKey{}, rctx)
 }
 
+func GetTykMode(ctx context.Context) environment.Env {
+	if c := ctx.Value(contextKey{}); c != nil {
+		if a, ok := c.(Context); ok {
+			return a.Env
+		}
+	}
+
+	return environment.Env{}
+}
+
 func GetContext(ctx context.Context) Context {
 	if c := ctx.Value(contextKey{}); c != nil {
 		return c.(Context)

diff --git a/pkg/client/gateway/security_policy.go b/pkg/client/gateway/security_policy.go
@@ -72,20 +72,14 @@ func (a SecurityPolicy) Create(ctx context.Context, def *v1.SecurityPolicySpec)
 			def.MID = new(string)
 		}
 
-		*def.MID = msg.Key
-
 		return nil
 	default:
 		return client.Error(res)
 	}
 }
 
 func (a SecurityPolicy) Update(ctx context.Context, def *v1.SecurityPolicySpec) error {
-	if def.MID == nil || *def.MID == "" {
-		return client.ErrMissingPolicyID
-	}
-
-	res, err := client.PutJSON(ctx, client.Join(endpointPolicies, *def.MID), def)
+	res, err := client.PutJSON(ctx, client.Join(endpointPolicies, *def.ID), def)
 	if err != nil {
 		return err
 	}