Skip to content

Commit

Permalink
feat: enforced condition overridden reason
Browse files Browse the repository at this point in the history
Closes: Kuadrant#349
  • Loading branch information
KevFan committed Feb 12, 2024
1 parent 336c81d commit 6569f1e
Show file tree
Hide file tree
Showing 5 changed files with 130 additions and 86 deletions.
4 changes: 2 additions & 2 deletions controllers/authpolicy_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
if delResErr == nil {
delResErr = err
}
return r.reconcileStatus(ctx, ap, common.NewErrTargetNotFound(ap.Kind(), ap.GetTargetRef(), delResErr))
return r.reconcileStatus(ctx, ap, targetNetworkObject, common.NewErrTargetNotFound(ap.Kind(), ap.GetTargetRef(), delResErr))
}
return ctrl.Result{}, err
}
Expand Down Expand Up @@ -103,7 +103,7 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ
specErr := r.reconcileResources(ctx, ap, targetNetworkObject)

// reconcile authpolicy status
statusResult, statusErr := r.reconcileStatus(ctx, ap, specErr)
statusResult, statusErr := r.reconcileStatus(ctx, ap, targetNetworkObject, specErr)

if specErr != nil {
return ctrl.Result{}, specErr
Expand Down
98 changes: 46 additions & 52 deletions controllers/authpolicy_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -310,58 +310,6 @@ var _ = Describe("AuthPolicy controller", func() {
Expect(authConfig.Spec.Conditions[0].Any[0].Any[0].All[1].Value).To(Equal("/.*"))
})

It("Attaches policy to the Gateway while having other policies attached to all HTTPRoutes", func() {
routePolicy := policyFactory()

err := k8sClient.Create(context.Background(), routePolicy)
logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err)
Expect(err).ToNot(HaveOccurred())

// check policy status
Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())

// attach policy to the gatewaay
gwPolicy := policyFactory(func(policy *api.AuthPolicy) {
policy.Name = "gw-auth"
policy.Spec.TargetRef.Group = "gateway.networking.k8s.io"
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = testGatewayName
})

err = k8sClient.Create(context.Background(), gwPolicy)
logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gwPolicy).String(), "error", err)
Expect(err).ToNot(HaveOccurred())

// check policy status
Eventually(func() bool {
existingPolicy := &api.AuthPolicy{}
err := k8sClient.Get(context.Background(), client.ObjectKeyFromObject(gwPolicy), existingPolicy)
if err != nil {
return false
}
condition := meta.FindStatusCondition(existingPolicy.Status.Conditions, string(common.PolicyConditionEnforced))
return condition != nil &&
condition.Reason == string(common.PolicyReasonUnknown) &&
condition.Message == "AuthPolicy has encountered some issues: AuthScheme is not ready yet"
}, 30*time.Second, 5*time.Second).Should(BeTrue())

// check istio authorizationpolicy
iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, gwPolicy.Spec.TargetRef), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), iapKey, &secv1beta1resources.AuthorizationPolicy{})
logf.Log.V(1).Info("Fetching Istio's AuthorizationPolicy", "key", iapKey.String(), "error", err)
return apierrors.IsNotFound(err)
}, 2*time.Minute, 5*time.Second).Should(BeTrue())

// check authorino authconfig
authConfigKey := types.NamespacedName{Name: authConfigName(client.ObjectKeyFromObject(gwPolicy)), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), authConfigKey, &authorinoapi.AuthConfig{})
return apierrors.IsNotFound(err)
}, 30*time.Second, 5*time.Second).Should(BeTrue())
})

It("Rejects policy with only unmatching top-level route selectors while trying to configure the gateway", func() {
policy := policyFactory(func(policy *api.AuthPolicy) {
policy.Spec.RouteSelectors = []api.RouteSelector{
Expand Down Expand Up @@ -1253,6 +1201,52 @@ var _ = Describe("AuthPolicy controller", func() {
Eventually(assertAcceptedCondTrueAndEnforcedCond(policy, metav1.ConditionFalse, string(common.PolicyReasonUnknown),
"AuthPolicy has encountered some issues: AuthScheme is not ready yet"), 30*time.Second, 5*time.Second).Should(BeTrue())
})

It("Overridden reason - Attaches policy to the Gateway while having other policies attached to all HTTPRoutes", func() {
routePolicy := policyFactory()

err := k8sClient.Create(context.Background(), routePolicy)
logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(routePolicy).String(), "error", err)
Expect(err).ToNot(HaveOccurred())

// check route policy status
Eventually(isAuthPolicyAccepted(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
Eventually(isAuthPolicyEnforced(routePolicy), 30*time.Second, 5*time.Second).Should(BeTrue())

// attach policy to the gatewaay
gwPolicy := policyFactory(func(policy *api.AuthPolicy) {
policy.Name = "gw-auth"
policy.Spec.TargetRef.Group = "gateway.networking.k8s.io"
policy.Spec.TargetRef.Kind = "Gateway"
policy.Spec.TargetRef.Name = testGatewayName
})

err = k8sClient.Create(context.Background(), gwPolicy)
logf.Log.V(1).Info("Creating AuthPolicy", "key", client.ObjectKeyFromObject(gwPolicy).String(), "error", err)
Expect(err).ToNot(HaveOccurred())

// check policy status
Eventually(isAuthPolicyAccepted(gwPolicy), 30*time.Second, 5*time.Second).Should(BeTrue())
Eventually(
assertAcceptedCondTrueAndEnforcedCond(gwPolicy, metav1.ConditionFalse, string(common.PolicyReasonOverridden),
fmt.Sprintf("AuthPolicy is overridden by [{\"Namespace\":\"%s\",\"Name\":\"%s\"}]", testNamespace, routePolicy.Name)),
30*time.Second, 5*time.Second).Should(BeTrue())

// check istio authorizationpolicy
iapKey := types.NamespacedName{Name: istioAuthorizationPolicyName(testGatewayName, gwPolicy.Spec.TargetRef), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), iapKey, &secv1beta1resources.AuthorizationPolicy{})
logf.Log.V(1).Info("Fetching Istio's AuthorizationPolicy", "key", iapKey.String(), "error", err)
return apierrors.IsNotFound(err)
}, 2*time.Minute, 5*time.Second).Should(BeTrue())

// check authorino authconfig
authConfigKey := types.NamespacedName{Name: authConfigName(client.ObjectKeyFromObject(gwPolicy)), Namespace: testNamespace}
Eventually(func() bool {
err := k8sClient.Get(context.Background(), authConfigKey, &authorinoapi.AuthConfig{})
return apierrors.IsNotFound(err)
}, 30*time.Second, 5*time.Second).Should(BeTrue())
})
})
})

Expand Down
87 changes: 57 additions & 30 deletions controllers/authpolicy_status.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package controllers

import (
"context"
"encoding/json"
"fmt"
"slices"

Expand All @@ -11,6 +12,7 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
ctrl "sigs.k8s.io/controller-runtime"
"sigs.k8s.io/controller-runtime/pkg/client"
v1 "sigs.k8s.io/gateway-api/apis/v1"
gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2"

authorinoapi "github.com/kuadrant/authorino/api/v1beta2"
Expand All @@ -19,32 +21,15 @@ import (
)

// reconcileStatus makes sure status block of AuthPolicy is up-to-date.
func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *api.AuthPolicy, specErr error) (ctrl.Result, error) {
func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, specErr error) (ctrl.Result, error) {
logger, _ := logr.FromContext(ctx)
logger.V(1).Info("Reconciling AuthPolicy status", "spec error", specErr)

// fetch the AuthConfig and check if it's ready.
isAuthConfigReady := true
if specErr == nil { // skip fetching authconfig if we already have a reconciliation error.
apKey := client.ObjectKeyFromObject(ap)
authConfigKey := client.ObjectKey{
Namespace: ap.Namespace,
Name: authConfigName(apKey),
}
authConfig := &authorinoapi.AuthConfig{}
if err := r.GetResource(ctx, authConfigKey, authConfig); err != nil && !apierrors.IsNotFound(err) {
return ctrl.Result{}, err
}

isAuthConfigReady = authConfig.Status.Ready()
}

newStatus := r.calculateStatus(ap, specErr, isAuthConfigReady)
newStatus := r.calculateStatus(ctx, ap, targetNetworkObject, specErr)

equalStatus := ap.Status.Equals(newStatus, logger)
logger.V(1).Info("Status", "status is different", !equalStatus)
logger.V(1).Info("Status", "generation is different", ap.Generation != ap.Status.ObservedGeneration)
logger.V(1).Info("Status", "AuthConfig is ready", isAuthConfigReady)
if equalStatus && ap.Generation == ap.Status.ObservedGeneration {
logger.V(1).Info("Status up-to-date. No changes required.")
return ctrl.Result{}, nil
Expand Down Expand Up @@ -72,7 +57,7 @@ func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *api.Auth
return ctrl.Result{}, nil
}

func (r *AuthPolicyReconciler) calculateStatus(ap *api.AuthPolicy, specErr error, authConfigReady bool) *api.AuthPolicyStatus {
func (r *AuthPolicyReconciler) calculateStatus(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, specErr error) *api.AuthPolicyStatus {
newStatus := &api.AuthPolicyStatus{
Conditions: slices.Clone(ap.Status.Conditions),
ObservedGeneration: ap.Status.ObservedGeneration,
Expand All @@ -86,27 +71,69 @@ func (r *AuthPolicyReconciler) calculateStatus(ap *api.AuthPolicy, specErr error
return newStatus
}

enforcedCond := r.enforcedCondition(ap, authConfigReady)
enforcedCond := r.enforcedCondition(ctx, ap, targetNetworkObject)
meta.SetStatusCondition(&newStatus.Conditions, *enforcedCond)

return newStatus
}

func (r *AuthPolicyReconciler) acceptedCondition(policy common.KuadrantPolicy, specErr error) *metav1.Condition {
cond := common.AcceptedCondition(policy, specErr)

return cond
return common.AcceptedCondition(policy, specErr)
}

func (r *AuthPolicyReconciler) enforcedCondition(policy common.KuadrantPolicy, authConfigReady bool) *metav1.Condition {
var err common.PolicyError
// enforcedCondition checks if the provided AuthPolicy is enforced based on the status of the associated AuthConfig and Gateway.
func (r *AuthPolicyReconciler) enforcedCondition(ctx context.Context, policy *api.AuthPolicy, targetNetworkObject client.Object) *metav1.Condition {
logger, _ := logr.FromContext(ctx)
authConfigReady, gwPolicyOverridden, err := r.checkAuthConfigAndGateway(ctx, policy)
if err != nil {
logger.Error(err, "Failed to check AuthConfig and Gateway")
return common.EnforcedCondition(policy, common.NewErrUnknown(policy.Kind(), err))
}

if !authConfigReady {
err = common.NewErrUnknown(policy.Kind(), fmt.Errorf("AuthScheme is not ready yet"))
logger.V(1).Info("AuthConfig is not ready")
return common.EnforcedCondition(policy, common.NewErrUnknown(policy.Kind(), fmt.Errorf("AuthScheme is not ready yet")))
}

// TODO: Implement 'Overridden' Reason if AuthPolicy supports Inherited Policy Attachment
if gwPolicyOverridden {
logger.V(1).Info("Gateway Policy is overridden")
return r.handleGatewayPolicyOverride(logger, policy, targetNetworkObject)
}

logger.V(1).Info("AuthPolicy is enforced")
return common.EnforcedCondition(policy, nil)
}

cond := common.EnforcedCondition(policy, err)
// checkAuthConfigAndGateway checks if the AuthConfig is ready and if the Gateway Policy is overridden.
func (r *AuthPolicyReconciler) checkAuthConfigAndGateway(ctx context.Context, policy *api.AuthPolicy) (bool, bool, error) {
apKey := client.ObjectKeyFromObject(policy)
authConfigKey := client.ObjectKey{
Namespace: policy.Namespace,
Name: authConfigName(apKey),
}
authConfig := &authorinoapi.AuthConfig{}
err := r.GetResource(ctx, authConfigKey, authConfig)
if err != nil {
if !apierrors.IsNotFound(err) {
return false, false, fmt.Errorf("failed to get AuthConfig: %w", err)
}
return true, common.IsTargetRefGateway(policy.GetTargetRef()), nil
}
return authConfig.Status.Ready(), false, nil
}

return cond
// handleGatewayPolicyOverride handles the case where the Gateway Policy is overridden.
func (r *AuthPolicyReconciler) handleGatewayPolicyOverride(logger logr.Logger, policy *api.AuthPolicy, targetNetworkObject client.Object) *metav1.Condition {
obj := targetNetworkObject.(*v1.Gateway)
gatewayWrapper := common.GatewayWrapper{Gateway: obj, PolicyRefsConfig: &common.KuadrantAuthPolicyRefsConfig{}}
refs := gatewayWrapper.PolicyRefs()
filteredRef := common.Filter(refs, func(key client.ObjectKey) bool {
return key != client.ObjectKeyFromObject(policy)
})
jsonData, err := json.Marshal(filteredRef)
if err != nil {
logger.Error(err, "Failed to marshal filtered references")
return common.EnforcedCondition(policy, common.NewErrUnknown(policy.Kind(), err))
}
return common.EnforcedCondition(policy, common.NewErrOverridden(policy.Kind(), string(jsonData)))
}
5 changes: 3 additions & 2 deletions pkg/common/apimachinery_status_conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import (
const (
PolicyConditionEnforced gatewayapiv1alpha2.PolicyConditionType = "Enforced"

PolicyReasonEnforced gatewayapiv1alpha2.PolicyConditionReason = "Enforced"
PolicyReasonUnknown gatewayapiv1alpha2.PolicyConditionReason = "Unknown"
PolicyReasonEnforced gatewayapiv1alpha2.PolicyConditionReason = "Enforced"
PolicyReasonOverridden gatewayapiv1alpha2.PolicyConditionReason = "Overridden"
PolicyReasonUnknown gatewayapiv1alpha2.PolicyConditionReason = "Unknown"
)

// ConditionMarshal marshals the set of conditions as a JSON array, sorted by condition type.
Expand Down
22 changes: 22 additions & 0 deletions pkg/common/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,3 +107,25 @@ func NewErrUnknown(kind string, err error) ErrUnknown {
Err: err,
}
}

var _ PolicyError = ErrOverridden{}

type ErrOverridden struct {
Kind string
OverridingPolicies string
}

func (e ErrOverridden) Error() string {
return fmt.Sprintf("%s is overridden by %s", e.Kind, e.OverridingPolicies)
}

func (e ErrOverridden) Reason() gatewayapiv1alpha2.PolicyConditionReason {
return PolicyReasonOverridden
}

func NewErrOverridden(kind, overridingPolicies string) ErrOverridden {
return ErrOverridden{
Kind: kind,
OverridingPolicies: overridingPolicies,
}
}

0 comments on commit 6569f1e

Please sign in to comment.