Skip to content

Commit

Permalink
fix: invalid reason not deleting second ns after test & fix missing c…
Browse files Browse the repository at this point in the history
…omments
  • Loading branch information
KevFan committed Apr 22, 2024
1 parent cb4c778 commit 85b6a96
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 9 deletions.
2 changes: 1 addition & 1 deletion controllers/authpolicy_authconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.Au
return mergeConditionsFromRouteSelectorsIntoConfigs(ap, route, authConfig)
}

// routeGatewayAuthOverrides returns the GW auth policies that has a
// routeGatewayAuthOverrides returns the GW auth policies that has an override field set
func routeGatewayAuthOverrides(t *kuadrantgatewayapi.Topology, ap *api.AuthPolicy) []client.ObjectKey {
affectedPolicies := getAffectedPolicies(t, ap)

Expand Down
16 changes: 11 additions & 5 deletions controllers/authpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
secv1beta1resources "istio.io/client-go/pkg/apis/security/v1beta1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -1136,21 +1137,26 @@ var _ = Describe("AuthPolicy controller", func() {
), 30*time.Second, 5*time.Second).Should(BeTrue())
})

It("Invalid reason", func() {
It("Invalid reason", func(ctx SpecContext) {
var otherNamespace string
CreateNamespace(&otherNamespace)
defer DeleteNamespaceCallback(&otherNamespace)
defer func(k8sClient client.Client, ctx context.Context, obj client.Object) {
err := k8sClient.Delete(ctx, obj)
if err != nil {
logf.Log.V(1).Info("failed to delete namespace", "namespace", obj.GetNamespace(), "name", obj.GetName())
}
}(k8sClient, ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: otherNamespace}})

policy := policyFactory(func(policy *api.AuthPolicy) {
policy.Namespace = otherNamespace // create the policy in a different namespace than the target
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(testGatewayName)
policy.Spec.TargetRef.Namespace = ptr.To(gatewayapiv1.Namespace(testNamespace))
})
Expect(k8sClient.Create(context.Background(), policy)).To(Succeed())
Expect(k8sClient.Create(ctx, policy)).To(Succeed())

Eventually(assertAcceptedCondFalseAndEnforcedCondNil(policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("AuthPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace)), 30*time.Second, 5*time.Second).Should(BeTrue())
})
Eventually(assertAcceptedCondFalseAndEnforcedCondNil(policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("AuthPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace))).WithContext(ctx).Should(BeTrue())
}, SpecTimeout(time.Minute))
})

Context("AuthPolicy enforced condition reasons", func() {
Expand Down
11 changes: 9 additions & 2 deletions controllers/ratelimitpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
appsv1 "k8s.io/api/apps/v1"
corev1 "k8s.io/api/core/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
logf "sigs.k8s.io/controller-runtime/pkg/log"
gatewayapiv1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

Expand Down Expand Up @@ -662,7 +664,12 @@ var _ = Describe("RateLimitPolicy controller", func() {
It("Invalid reason", func(ctx SpecContext) {
var otherNamespace string
CreateNamespace(&otherNamespace)
defer DeleteNamespaceCallback(&otherNamespace)
defer func(k8sClient client.Client, ctx context.Context, obj client.Object) {
err := k8sClient.Delete(ctx, obj)
if err != nil {
logf.Log.V(1).Info("failed to delete namespace", "namespace", obj.GetNamespace(), "name", obj.GetName())
}
}(k8sClient, ctx, &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: otherNamespace}})

policy := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Namespace = otherNamespace // create the policy in a different namespace than the target
Expand All @@ -672,7 +679,7 @@ var _ = Describe("RateLimitPolicy controller", func() {
})
Expect(k8sClient.Create(ctx, policy)).To(Succeed())

Eventually(assertAcceptedConditionFalse(ctx, policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("RateLimitPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace)), 30*time.Second, 5*time.Second).Should(BeTrue())
Eventually(assertAcceptedConditionFalse(ctx, policy, string(gatewayapiv1alpha2.PolicyReasonInvalid), fmt.Sprintf("RateLimitPolicy target is invalid: invalid targetRef.Namespace %s. Currently only supporting references to the same namespace", testNamespace))).WithContext(ctx).Should(Succeed())
}, SpecTimeout(time.Minute))
})

Expand Down
2 changes: 1 addition & 1 deletion controllers/ratelimitpolicy_limits.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ func (r *RateLimitPolicyReconciler) getAffectedPoliciesInfo(rlp *kuadrantv1beta2
}

// applyGatewayOverrides a Gateway RLP is not affected if there is untargetted routes or affects other policies
// Otherwise, it
// Otherwise, it has no free routes, and therefore "overridden" by the route policies
func (r *RateLimitPolicyReconciler) applyGatewayOverrides(logger logr.Logger, rlp *kuadrantv1beta2.RateLimitPolicy, numUnTargetedRoutes int, affectedPolicies []kuadrantgatewayapi.Policy) {
if rlp.Spec.Overrides == nil && numUnTargetedRoutes == 0 {
r.AffectedPolicyMap.SetAffectedPolicy(rlp, utils.Map(affectedPolicies, func(p kuadrantgatewayapi.Policy) client.ObjectKey {
Expand Down

0 comments on commit 85b6a96

Please sign in to comment.