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.

Sign up for GitHub

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

Jump to bottom

Fix dependencies namespace propagation #109

Merged
merged 9 commits into from
Dec 8, 2022
Merged

Fix dependencies namespace propagation #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2e2656d
[go] Adding sync lib
didierofrivia Nov 22, 2022
6572f30
[controller] Reconciling GWs
didierofrivia Nov 22, 2022
75df323
[policies] Helper to get the Kuadrant NS from KAP and RLP
didierofrivia Nov 24, 2022
a5ddc69
[kap controller] Replacing the hardcoded value in AuthPolicy controller
didierofrivia Nov 24, 2022
ee35727
[rlp controller] Replacing hardcoded values in RateLimit controller
didierofrivia Nov 24, 2022
d098e03
[common] Removing unused constants and env vars
didierofrivia Nov 28, 2022
e5fcce4
[kuadrant] Using LimitadorName constant
didierofrivia Nov 28, 2022
34888de
[rlc] Using common.LimitadorName instead of removed rlptools
didierofrivia Nov 28, 2022
4f94832
[test] Waiting longer for tearing down the project
didierofrivia Dec 5, 2022
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
8 changes: 8 additions & 0 deletions api/v1beta1/authpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,11 @@ func (ap *AuthPolicy) Validate() error {
}
return nil
}

func (ap *AuthPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
return ap.Spec.TargetRef
}

func (ap *AuthPolicy) GetWrappedNamespace() gatewayapiv1alpha2.Namespace {
return gatewayapiv1alpha2.Namespace(ap.Namespace)
}
8 changes: 8 additions & 0 deletions api/v1beta1/ratelimitpolicy_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,14 @@ type RateLimitPolicyList struct {
Items []RateLimitPolicy `json:"items"`
}

func (r *RateLimitPolicy) GetTargetRef() gatewayapiv1alpha2.PolicyTargetReference {
return r.Spec.TargetRef
}

func (r *RateLimitPolicy) GetWrappedNamespace() gatewayapiv1alpha2.Namespace {
return gatewayapiv1alpha2.Namespace(r.Namespace)
}

func init() {
SchemeBuilder.Register(&RateLimitPolicy{}, &RateLimitPolicyList{})
}
42 changes: 30 additions & 12 deletions controllers/authpolicy_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,32 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
return ctrl.Result{}, err
}

logger.V(1).Info("Getting Kuadrant namespace")
var kuadrantNamespace string
kuadrantNamespace, isSet := common.GetNamespaceFromPolicy(&ap)
if !isSet {
var err error
kuadrantNamespace, err = common.GetNamespaceFromPolicyTargetRef(ctx, r.Client(), &ap)
if err != nil {
logger.Error(err, "failed to get Kuadrant namespace")
return ctrl.Result{}, err
}
common.AnnotateObject(&ap, kuadrantNamespace)
err = r.UpdateResource(ctx, &ap)
if err != nil {
logger.Error(err, "failed to update policy, re-queuing")
return ctrl.Result{Requeue: true}, err
}
}

if ap.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(&ap, authPolicyFinalizer) {
logger.V(1).Info("Handling removal of authpolicy object")
if err := r.removeIstioAuthPolicy(ctx, &ap); err != nil {
logger.Error(err, "failed to remove Istio's AuthorizationPolicy")
return ctrl.Result{}, err
}

if err := r.removeAuthSchemes(ctx, &ap); err != nil {
if err := r.removeAuthSchemes(ctx, &ap, kuadrantNamespace); err != nil {
return ctrl.Result{}, err
}

Expand Down Expand Up @@ -88,13 +106,13 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
}
}

specResult, specErr := r.reconcileSpec(ctx, &ap)
specResult, specErr := r.reconcileSpec(ctx, &ap, kuadrantNamespace)
if specErr == nil && specResult.Requeue {
logger.V(1).Info("Reconciling spec not finished. Requeueing.")
return specResult, nil
}

statusResult, statusErr := r.reconcileStatus(ctx, &ap, specErr)
statusResult, statusErr := r.reconcileStatus(ctx, &ap, kuadrantNamespace, specErr)

if specErr != nil {
return ctrl.Result{}, specErr
Expand All @@ -113,7 +131,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
return ctrl.Result{}, nil
}

func (r *AuthPolicyReconciler) reconcileSpec(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) (ctrl.Result, error) {
func (r *AuthPolicyReconciler) reconcileSpec(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) (ctrl.Result, error) {
if err := ap.Validate(); err != nil {
return ctrl.Result{}, err
}
Expand All @@ -130,7 +148,7 @@ func (r *AuthPolicyReconciler) reconcileSpec(ctx context.Context, ap *kuadrantv1
return ctrl.Result{}, err
}

if err := r.reconcileAuthSchemes(ctx, ap); err != nil {
if err := r.reconcileAuthSchemes(ctx, ap, kuadrantNamespace); err != nil {
return ctrl.Result{}, err
}

Expand All @@ -155,13 +173,13 @@ func (r *AuthPolicyReconciler) enforceHierarchicalConstraints(ctx context.Contex
return nil
}

func (r *AuthPolicyReconciler) reconcileAuthSchemes(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error {
func (r *AuthPolicyReconciler) reconcileAuthSchemes(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) error {
logger, err := logr.FromContext(ctx)
if err != nil {
return err
}

authConfig, err := r.desiredAuthConfig(ctx, ap)
authConfig, err := r.desiredAuthConfig(ctx, ap, kuadrantNamespace)
if err != nil {
return err
}
Expand Down Expand Up @@ -274,7 +292,7 @@ func (r *AuthPolicyReconciler) reconcileNetworkResourceBackReference(ctx context
ap.Namespace, common.AuthPolicyBackRefAnnotation)
}

func (r *AuthPolicyReconciler) removeAuthSchemes(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error {
func (r *AuthPolicyReconciler) removeAuthSchemes(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) error {
logger, err := logr.FromContext(ctx)
if err != nil {
return err
Expand All @@ -290,7 +308,7 @@ func (r *AuthPolicyReconciler) removeAuthSchemes(ctx context.Context, ap *kuadra
},
ObjectMeta: metav1.ObjectMeta{
Name: authConfigName(apKey),
Namespace: common.KuadrantNamespace,
Namespace: kuadrantNamespace,
},
}

Expand All @@ -305,7 +323,7 @@ func (r *AuthPolicyReconciler) removeAuthSchemes(ctx context.Context, ap *kuadra
return nil
}

func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) (*authorinov1beta1.AuthConfig, error) {
func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) (*authorinov1beta1.AuthConfig, error) {
hosts, err := r.policyHosts(ctx, ap)
if err != nil {
return nil, err
Expand All @@ -318,7 +336,7 @@ func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *kuadra
},
ObjectMeta: metav1.ObjectMeta{
Name: authConfigName(client.ObjectKeyFromObject(ap)),
Namespace: common.KuadrantNamespace,
Namespace: kuadrantNamespace,
},
Spec: authorinov1beta1.AuthConfigSpec{
Hosts: hosts,
Expand Down Expand Up @@ -380,7 +398,7 @@ func (r *AuthPolicyReconciler) removeIstioAuthPolicy(ctx context.Context, ap *ku
},
}

if err := r.DeleteResource(ctx, istioAuthPolicy); err != nil {
if err := r.DeleteResource(ctx, istioAuthPolicy); err != nil && !apierrors.IsNotFound(err) {
logger.Error(err, "failed to delete Istio's AuthorizationPolicy")
return err
}
Expand Down
98 changes: 56 additions & 42 deletions controllers/authpolicy_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,20 +21,26 @@ import (
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1"
"github.com/kuadrant/kuadrant-operator/pkg/common"
)

const GatewayName = "istio-ingressgateway"
const (
IstioGatewayName = "istio-ingressgateway"
IstioGatewayNamespace = "istio-system"
CustomGatewayName = "toystore-gw"
)

var _ = Describe("AuthPolicy controller", func() {
var (
testNamespace string
routeName = "toystore-route"
gwName = "toystore-gw"
gwName = CustomGatewayName
)

beforeEachCallback := func() {
CreateNamespace(&testNamespace)
gateway := testBuildBasicGateway(gwName, testNamespace)
err := k8sClient.Create(context.Background(), gateway)
Expect(err).ToNot(HaveOccurred())
ApplyKuadrantCR(testNamespace)
}

Expand All @@ -60,9 +66,15 @@ var _ = Describe("AuthPolicy controller", func() {

// check Istio's AuthorizationPolicy existence
iap := &secv1beta1resources.AuthorizationPolicy{}
namespace := IstioGatewayNamespace
name := IstioGatewayName
if authpolicies[idx].Spec.TargetRef.Kind == "Gateway" {
namespace = testNamespace
name = CustomGatewayName
}
iapKey := types.NamespacedName{
Name: getAuthPolicyName(GatewayName, string(authpolicies[idx].Spec.TargetRef.Name), string(authpolicies[idx].Spec.TargetRef.Kind)),
Namespace: "istio-system",
Name: getAuthPolicyName(name, string(authpolicies[idx].Spec.TargetRef.Name), string(authpolicies[idx].Spec.TargetRef.Kind)),
Namespace: namespace,
}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), iapKey, iap)
Expand All @@ -77,7 +89,7 @@ var _ = Describe("AuthPolicy controller", func() {
ac := &authorinov1beta1.AuthConfig{}
acKey := types.NamespacedName{
Name: authConfigName(client.ObjectKeyFromObject(authpolicies[idx])),
Namespace: common.KuadrantNamespace,
Namespace: testNamespace,
}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), acKey, ac)
Expand All @@ -97,9 +109,15 @@ var _ = Describe("AuthPolicy controller", func() {

// check Istio's AuthorizationPolicy existence
iap := &secv1beta1resources.AuthorizationPolicy{}
namespace := IstioGatewayNamespace
name := IstioGatewayName
if authpolicies[idx].Spec.TargetRef.Kind == "Gateway" {
namespace = testNamespace
name = CustomGatewayName
}
iapKey := types.NamespacedName{
Name: getAuthPolicyName(GatewayName, string(authpolicies[idx].Spec.TargetRef.Name), string(authpolicies[idx].Spec.TargetRef.Kind)),
Namespace: common.KuadrantNamespace,
Name: getAuthPolicyName(name, string(authpolicies[idx].Spec.TargetRef.Name), string(authpolicies[idx].Spec.TargetRef.Kind)),
Namespace: namespace,
}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), iapKey, iap)
Expand All @@ -114,7 +132,7 @@ var _ = Describe("AuthPolicy controller", func() {
ac := &authorinov1beta1.AuthConfig{}
acKey := types.NamespacedName{
Name: authConfigName(client.ObjectKeyFromObject(authpolicies[idx])),
Namespace: common.KuadrantNamespace,
Namespace: testNamespace,
}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), acKey, ac)
Expand All @@ -131,24 +149,22 @@ var _ = Describe("AuthPolicy controller", func() {

Context("Some rules without hosts", func() {
BeforeEach(func() {
gateway := testBuildBasicGateway(gwName, testNamespace)
err := k8sClient.Create(context.Background(), gateway)
Expect(err).ToNot(HaveOccurred())

httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.toystore.com"})
err = k8sClient.Create(context.Background(), httpRoute)
err := k8sClient.Create(context.Background(), httpRoute)
Expect(err).ToNot(HaveOccurred())

typedNamespace := v1alpha2.Namespace(testNamespace)
policy := &kuadrantv1beta1.AuthPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "toystore",
Namespace: testNamespace,
},
Spec: kuadrantv1beta1.AuthPolicySpec{
TargetRef: v1alpha2.PolicyTargetReference{
Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
Namespace: &typedNamespace,
},
AuthRules: []kuadrantv1beta1.AuthRule{
{
Expand Down Expand Up @@ -188,7 +204,7 @@ var _ = Describe("AuthPolicy controller", func() {
// Check authconfig's hosts
kapKey := client.ObjectKey{Name: "toystore", Namespace: testNamespace}
existingAuthC := &authorinov1beta1.AuthConfig{}
authCKey := types.NamespacedName{Name: authConfigName(kapKey), Namespace: common.KuadrantNamespace}
authCKey := types.NamespacedName{Name: authConfigName(kapKey), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), authCKey, existingAuthC)
return err == nil
Expand Down Expand Up @@ -226,24 +242,22 @@ var _ = Describe("AuthPolicy controller", func() {

Context("All rules with subdomains", func() {
BeforeEach(func() {
gateway := testBuildBasicGateway(gwName, testNamespace)
err := k8sClient.Create(context.Background(), gateway)
Expect(err).ToNot(HaveOccurred())

httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.toystore.com"})
err = k8sClient.Create(context.Background(), httpRoute)
err := k8sClient.Create(context.Background(), httpRoute)
Expect(err).ToNot(HaveOccurred())

typedNamespace := v1alpha2.Namespace(testNamespace)
policy := &kuadrantv1beta1.AuthPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "toystore",
Namespace: testNamespace,
},
Spec: kuadrantv1beta1.AuthPolicySpec{
TargetRef: v1alpha2.PolicyTargetReference{
Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
Namespace: &typedNamespace,
},
AuthRules: []kuadrantv1beta1.AuthRule{
{
Expand Down Expand Up @@ -289,7 +303,7 @@ var _ = Describe("AuthPolicy controller", func() {
// Check authconfig's hosts
kapKey := client.ObjectKey{Name: "toystore", Namespace: testNamespace}
existingAuthC := &authorinov1beta1.AuthConfig{}
authCKey := types.NamespacedName{Name: authConfigName(kapKey), Namespace: common.KuadrantNamespace}
authCKey := types.NamespacedName{Name: authConfigName(kapKey), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), authCKey, existingAuthC)
return err == nil
Expand All @@ -301,24 +315,22 @@ var _ = Describe("AuthPolicy controller", func() {

Context("No rules", func() {
BeforeEach(func() {
gateway := testBuildBasicGateway(gwName, testNamespace)
err := k8sClient.Create(context.Background(), gateway)
Expect(err).ToNot(HaveOccurred())

httpRoute := testBuildBasicHttpRoute(routeName, gwName, testNamespace, []string{"*.toystore.com"})
err = k8sClient.Create(context.Background(), httpRoute)
err := k8sClient.Create(context.Background(), httpRoute)
Expect(err).ToNot(HaveOccurred())

typedNamespace := v1alpha2.Namespace(testNamespace)
policy := &kuadrantv1beta1.AuthPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "toystore",
Namespace: testNamespace,
},
Spec: kuadrantv1beta1.AuthPolicySpec{
TargetRef: v1alpha2.PolicyTargetReference{
Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group),
Kind: "HTTPRoute",
Name: gatewayapiv1alpha2.ObjectName(routeName),
Namespace: &typedNamespace,
},
AuthRules: nil,
AuthScheme: testBasicAuthScheme(),
Expand Down Expand Up @@ -347,7 +359,7 @@ var _ = Describe("AuthPolicy controller", func() {
// Check authconfig's hosts
kapKey := client.ObjectKey{Name: "toystore", Namespace: testNamespace}
existingAuthC := &authorinov1beta1.AuthConfig{}
authCKey := types.NamespacedName{Name: authConfigName(kapKey), Namespace: common.KuadrantNamespace}
authCKey := types.NamespacedName{Name: authConfigName(kapKey), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), authCKey, existingAuthC)
return err == nil
Expand Down Expand Up @@ -381,16 +393,18 @@ func testBasicAuthScheme() kuadrantv1beta1.AuthSchemeSpec {
}

func authPolicies(namespace string) []*kuadrantv1beta1.AuthPolicy {
typedNamespace := v1alpha2.Namespace(namespace)
routePolicy := &kuadrantv1beta1.AuthPolicy{
ObjectMeta: metav1.ObjectMeta{
Name: "target-route",
Namespace: namespace,
},
Spec: kuadrantv1beta1.AuthPolicySpec{
TargetRef: v1alpha2.PolicyTargetReference{
Group: "gateway.networking.k8s.io",
Kind: "HTTPRoute",
Name: "toystore",
Group: "gateway.networking.k8s.io",
Kind: "HTTPRoute",
Name: "toystore",
Namespace: &typedNamespace,
},
AuthRules: []kuadrantv1beta1.AuthRule{
{
Expand Down Expand Up @@ -421,12 +435,12 @@ func authPolicies(namespace string) []*kuadrantv1beta1.AuthPolicy {
},
},
}

gatewayPolicy := routePolicy.DeepCopy()
gatewayPolicy.SetName("target-gateway")
gatewayPolicy.SetNamespace("istio-system")
gatewayPolicy.SetNamespace(namespace)
gatewayPolicy.Spec.TargetRef.Kind = "Gateway"
gatewayPolicy.Spec.TargetRef.Name = GatewayName
gatewayPolicy.Spec.TargetRef.Name = CustomGatewayName
gatewayPolicy.Spec.TargetRef.Namespace = &typedNamespace
gatewayPolicy.Spec.AuthRules = []kuadrantv1beta1.AuthRule{
{Hosts: []string{"*.toystore.com"}},
}
Expand Down
Loading