Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

use correct status type when enforcing policy #594

Merged
merged 1 commit into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 31 additions & 36 deletions controllers/authpolicy_controller_test.go

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions controllers/dnspolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,9 @@ var _ = Describe("DNSPolicy controller", func() {
}),
MatchFields(IgnoreExtras, Fields{
"Type": Equal(string(kuadrant.PolicyConditionEnforced)),
"Status": Equal(metav1.ConditionTrue),
"Status": Equal(metav1.ConditionFalse),
"Reason": Equal(string(kuadrant.PolicyReasonEnforced)),
"Message": Equal("DNSPolicy has been successfully enforced"),
"Message": Equal("DNSPolicy has been partially enforced"),
})),
)
}, TestTimeoutMedium, time.Second).Should(Succeed())
Expand Down
2 changes: 1 addition & 1 deletion controllers/dnspolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@
controlled = true
// if at least one record not ready the policy is not enforced
for _, condition := range record.Status.Conditions {
if condition.Type == string(v1alpha2.PolicyConditionAccepted) && condition.Status == metav1.ConditionFalse {
if condition.Type == string(kuadrantdnsv1alpha1.ConditionTypeReady) && condition.Status == metav1.ConditionFalse {

Check warning on line 118 in controllers/dnspolicy_status.go

View check run for this annotation

Codecov / codecov/patch

controllers/dnspolicy_status.go#L118

Added line #L118 was not covered by tests
return kuadrant.EnforcedCondition(dnsPolicy, nil, false)
}
}
Expand Down
1 change: 1 addition & 0 deletions controllers/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ const (
TestListenerNameTwo = "test-listener-2"
TestIPAddressOne = "172.0.0.1"
TestIPAddressTwo = "172.0.0.2"
TestHTTPRouteName = "toystore-route"
)

func ApplyKuadrantCR(namespace string) {
Expand Down
7 changes: 3 additions & 4 deletions controllers/limitador_cluster_envoyfilter_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ var _ = Describe("Limitador Cluster EnvoyFilter controller", Ordered, func() {
var (
testNamespace string
kuadrantInstallationNS string
gwName = "toystore-gw"
rlpName = "toystore-rlp"
efName = fmt.Sprintf("kuadrant-ratelimiting-cluster-%s", gwName)
efName = fmt.Sprintf("kuadrant-ratelimiting-cluster-%s", TestGatewayName)
)

BeforeAll(func(ctx SpecContext) {
Expand All @@ -47,7 +46,7 @@ var _ = Describe("Limitador Cluster EnvoyFilter controller", Ordered, func() {

beforeEachCallback := func(ctx SpecContext) {
testNamespace = CreateNamespaceWithContext(ctx)
gateway := testBuildBasicGateway(gwName, testNamespace)
gateway := testBuildBasicGateway(TestGatewayName, testNamespace)
err := k8sClient.Create(ctx, gateway)
Expect(err).ToNot(HaveOccurred())

Expand Down Expand Up @@ -102,7 +101,7 @@ var _ = Describe("Limitador Cluster EnvoyFilter controller", Ordered, func() {
TargetRef: gatewayapiv1alpha2.PolicyTargetReference{
Group: gatewayapiv1.GroupName,
Kind: "Gateway",
Name: gatewayapiv1.ObjectName(gwName),
Name: gatewayapiv1.ObjectName(TestGatewayName),
},
RateLimitPolicyCommonSpec: kuadrantv1beta2.RateLimitPolicyCommonSpec{
Limits: map[string]kuadrantv1beta2.Limit{
Expand Down
71 changes: 31 additions & 40 deletions controllers/rate_limiting_wasmplugin_controller_test.go

Large diffs are not rendered by default.

47 changes: 23 additions & 24 deletions controllers/ratelimitpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
testNamespace string
kuadrantInstallationNS string
routeName = "toystore-route"
gwName = "toystore-gw"
rlpName = "toystore-rlp"
gateway *gatewayapiv1.Gateway
)
Expand Down Expand Up @@ -99,7 +98,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {

beforeEachCallback := func(ctx SpecContext) {
testNamespace = CreateNamespaceWithContext(ctx)
gateway = testBuildBasicGateway(gwName, testNamespace)
gateway = testBuildBasicGateway(TestGatewayName, testNamespace)

Expect(k8sClient.Create(ctx, gateway)).To(Succeed())
Eventually(testGatewayIsReady(gateway)).WithContext(ctx).Should(BeTrue())
Expand All @@ -122,7 +121,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
Context("RLP targeting HTTPRoute", func() {
It("Creates all the resources for a basic HTTPRoute and RateLimitPolicy", func(ctx SpecContext) {
// create httproute
httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
httpRoute := testBuildBasicHttpRoute(routeName, TestGatewayName, testNamespace, []string{"*.example.com"})
err := k8sClient.Create(ctx, httpRoute)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue())
Expand Down Expand Up @@ -163,7 +162,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
}))

// Check gateway back references
gwKey := client.ObjectKey{Name: gwName, Namespace: testNamespace}
gwKey := client.ObjectKey{Name: TestGatewayName, Namespace: testNamespace}
existingGateway := &gatewayapiv1.Gateway{}
Eventually(func(g Gomega) {
err = k8sClient.Get(ctx, gwKey, existingGateway)
Expand All @@ -181,7 +180,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
Context("RLP targeting Gateway", func() {
It("Creates all the resources for a basic Gateway and RateLimitPolicy", func(ctx SpecContext) {
// create httproute
httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
httpRoute := testBuildBasicHttpRoute(routeName, TestGatewayName, testNamespace, []string{"*.example.com"})
err := k8sClient.Create(ctx, httpRoute)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue())
Expand All @@ -200,7 +199,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
TargetRef: gatewayapiv1alpha2.PolicyTargetReference{
Group: gatewayapiv1.Group("gateway.networking.k8s.io"),
Kind: "Gateway",
Name: gatewayapiv1.ObjectName(gwName),
Name: gatewayapiv1.ObjectName(TestGatewayName),
},
Defaults: &kuadrantv1beta2.RateLimitPolicyCommonSpec{
Limits: map[string]kuadrantv1beta2.Limit{
Expand Down Expand Up @@ -264,7 +263,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// create ratelimitpolicy
rlp := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName)
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(TestGatewayName)
})
err := k8sClient.Create(ctx, rlp)
Expect(err).ToNot(HaveOccurred())
Expand Down Expand Up @@ -323,14 +322,14 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// GW policy defaults are overridden and not enforced when Route has their own policy attached

// create httproute
httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
httpRoute := testBuildBasicHttpRoute(routeName, TestGatewayName, testNamespace, []string{"*.example.com"})
Expect(k8sClient.Create(ctx, httpRoute)).To(Succeed())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue())

// create GW RLP
gwRLP = policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName)
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(TestGatewayName)
})
Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed())
gwRLPKey := client.ObjectKey{Name: gwRLP.Name, Namespace: testNamespace}
Expand Down Expand Up @@ -385,7 +384,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {

When("Free route is created", func() {
It("Gateway policy should now be enforced", func(ctx SpecContext) {
route2 := testBuildBasicHttpRoute("route2", gwName, testNamespace, []string{"*.car.com"})
route2 := testBuildBasicHttpRoute("route2", TestGatewayName, testNamespace, []string{"*.car.com"})
Expect(k8sClient.Create(ctx, route2)).To(Succeed())
Eventually(testRLPIsEnforced(ctx, client.ObjectKeyFromObject(gwRLP))).WithContext(ctx).Should(BeTrue())
}, testTimeOut)
Expand All @@ -402,7 +401,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
It("Explicit defaults - no underlying routes to enforce policy", func(ctx SpecContext) {
gwRLP := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName)
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(TestGatewayName)
})

Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed())
Expand All @@ -414,7 +413,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
It("Implicit defaults - no underlying routes to enforce policy", func(ctx SpecContext) {
gwRLP := policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName)
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(TestGatewayName)
policy.Spec.RateLimitPolicyCommonSpec = *policy.Spec.Defaults.DeepCopy()
policy.Spec.Defaults = nil
})
Expand All @@ -432,13 +431,13 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {

BeforeEach(func(ctx SpecContext) {
// create httproute
httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
httpRoute := testBuildBasicHttpRoute(routeName, TestGatewayName, testNamespace, []string{"*.example.com"})
Expect(k8sClient.Create(ctx, httpRoute)).To(Succeed())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue())

gwRLP = policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName)
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(TestGatewayName)
policy.Spec.Overrides = policy.Spec.Defaults.DeepCopy()
policy.Spec.Defaults = nil
})
Expand Down Expand Up @@ -564,7 +563,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// Create GW RLP with defaults
gwRLP = policyFactory(func(policy *kuadrantv1beta2.RateLimitPolicy) {
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(gwName)
policy.Spec.TargetRef.Name = gatewayapiv1.ObjectName(TestGatewayName)
})
Expect(k8sClient.Create(ctx, gwRLP)).To(Succeed())
gwRLPKey := client.ObjectKeyFromObject(gwRLP)
Expand Down Expand Up @@ -707,7 +706,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
}, testTimeOut)

It("Conflict reason", func(ctx SpecContext) {
httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
httpRoute := testBuildBasicHttpRoute(routeName, TestGatewayName, testNamespace, []string{"*.example.com"})
Expect(k8sClient.Create(ctx, httpRoute)).To(Succeed())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue())

Expand Down Expand Up @@ -764,7 +763,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
}

BeforeEach(func(ctx SpecContext) {
route := testBuildBasicHttpRoute(testHTTPRouteName, testGatewayName, testNamespace, []string{"*.toystore.com"})
route := testBuildBasicHttpRoute(TestHTTPRouteName, TestGatewayName, testNamespace, []string{"*.toystore.com"})
Expect(k8sClient.Create(ctx, route)).To(Succeed())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(route))).WithContext(ctx).Should(BeTrue())
})
Expand Down Expand Up @@ -817,7 +816,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// RLP A -> Route B

// create httproute A
httpRouteA := testBuildBasicHttpRoute(routeAName, gwName, testNamespace, []string{"*.a.example.com"})
httpRouteA := testBuildBasicHttpRoute(routeAName, TestGatewayName, testNamespace, []string{"*.a.example.com"})
err := k8sClient.Create(ctx, httpRouteA)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteA))).WithContext(ctx).Should(BeTrue())
Expand Down Expand Up @@ -847,7 +846,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// To RLP A -> Route B

// create httproute B
httpRouteB := testBuildBasicHttpRoute(routeBName, gwName, testNamespace, []string{"*.b.example.com"})
httpRouteB := testBuildBasicHttpRoute(routeBName, TestGatewayName, testNamespace, []string{"*.b.example.com"})
err = k8sClient.Create(ctx, httpRouteB)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteB))).WithContext(ctx).Should(BeTrue())
Expand Down Expand Up @@ -974,13 +973,13 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// RLP B -> Route B

// create httproute A
httpRouteA := testBuildBasicHttpRoute(routeAName, gwName, testNamespace, []string{"*.a.example.com"})
httpRouteA := testBuildBasicHttpRoute(routeAName, TestGatewayName, testNamespace, []string{"*.a.example.com"})
err := k8sClient.Create(ctx, httpRouteA)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteA))).WithContext(ctx).Should(BeTrue())

// create httproute B
httpRouteB := testBuildBasicHttpRoute(routeBName, gwName, testNamespace, []string{"*.b.example.com"})
httpRouteB := testBuildBasicHttpRoute(routeBName, TestGatewayName, testNamespace, []string{"*.b.example.com"})
err = k8sClient.Create(ctx, httpRouteB)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteB))).WithContext(ctx).Should(BeTrue())
Expand Down Expand Up @@ -1068,7 +1067,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// RLP A

// create httproute A
httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.example.com"})
httpRoute := testBuildBasicHttpRoute(routeName, TestGatewayName, testNamespace, []string{"*.example.com"})
err := k8sClient.Create(ctx, httpRoute)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRoute))).WithContext(ctx).Should(BeTrue())
Expand Down Expand Up @@ -1128,7 +1127,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// RLP B -> Route A

// create httproute A
httpRouteA := testBuildBasicHttpRoute(routeAName, gwName, testNamespace, []string{"*.a.example.com"})
httpRouteA := testBuildBasicHttpRoute(routeAName, TestGatewayName, testNamespace, []string{"*.a.example.com"})
err := k8sClient.Create(ctx, httpRouteA)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteA))).WithContext(ctx).Should(BeTrue())
Expand Down Expand Up @@ -1175,7 +1174,7 @@ var _ = Describe("RateLimitPolicy controller", Ordered, func() {
// RLP A was the older owner of route A, and wiil be the new owner of route B

// create httproute B
httpRouteB := testBuildBasicHttpRoute(routeBName, gwName, testNamespace, []string{"*.b.example.com"})
httpRouteB := testBuildBasicHttpRoute(routeBName, TestGatewayName, testNamespace, []string{"*.b.example.com"})
err = k8sClient.Create(ctx, httpRouteB)
Expect(err).ToNot(HaveOccurred())
Eventually(testRouteIsAccepted(client.ObjectKeyFromObject(httpRouteB))).WithContext(ctx).Should(BeTrue())
Expand Down
2 changes: 2 additions & 0 deletions controllers/ratelimitpolicy_status_test.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
//go:build unit

package controllers

import (
Expand Down
Loading
Loading