diff --git a/.gitignore b/.gitignore index 780fb3c02..50789c52e 100644 --- a/.gitignore +++ b/.gitignore @@ -25,6 +25,7 @@ testbin/* *.swo *~ kuadrant-operator +tmp /catalog/kuadrant-operator-catalog /catalog/kuadrant-operator-catalog.Dockerfile diff --git a/controllers/authpolicy_auth_config.go b/controllers/authpolicy_auth_config.go new file mode 100644 index 000000000..69216fbb1 --- /dev/null +++ b/controllers/authpolicy_auth_config.go @@ -0,0 +1,137 @@ +package controllers + +import ( + "context" + "fmt" + "reflect" + + "github.com/go-logr/logr" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + + authorinoapi "github.com/kuadrant/authorino/api/v1beta1" + api "github.com/kuadrant/kuadrant-operator/api/v1beta1" +) + +func (r *AuthPolicyReconciler) reconcileAuthConfigs(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { + logger, err := logr.FromContext(ctx) + if err != nil { + return err + } + + authConfig, err := r.desiredAuthConfig(ctx, ap, targetNetworkObject) + if err != nil { + return err + } + + err = r.ReconcileResource(ctx, &authorinoapi.AuthConfig{}, authConfig, alwaysUpdateAuthConfig) + if err != nil && !apierrors.IsAlreadyExists(err) { + logger.Error(err, "ReconcileResource failed to create/update AuthConfig resource") + return err + } + return nil +} + +func (r *AuthPolicyReconciler) deleteAuthConfigs(ctx context.Context, ap *api.AuthPolicy) error { + logger, err := logr.FromContext(ctx) + if err != nil { + return err + } + + logger.Info("Removing Authorino's AuthConfigs") + + authConfig := &authorinoapi.AuthConfig{ + ObjectMeta: metav1.ObjectMeta{ + Name: authConfigName(client.ObjectKeyFromObject(ap)), + Namespace: ap.Namespace, + }, + } + + if err := r.DeleteResource(ctx, authConfig); err != nil { + if apierrors.IsNotFound(err) { + return nil + } + logger.Error(err, "failed to delete Authorino's AuthConfig") + return err + } + + return nil +} + +func (r *AuthPolicyReconciler) desiredAuthConfig(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) (*authorinoapi.AuthConfig, error) { + hosts, err := r.policyHosts(ctx, ap, targetNetworkObject) + if err != nil { + return nil, err + } + + return &authorinoapi.AuthConfig{ + TypeMeta: metav1.TypeMeta{ + Kind: "AuthConfig", + APIVersion: authorinoapi.GroupVersion.String(), + }, + ObjectMeta: metav1.ObjectMeta{ + Name: authConfigName(client.ObjectKeyFromObject(ap)), + Namespace: ap.Namespace, + }, + Spec: authorinoapi.AuthConfigSpec{ + Hosts: hosts, + Patterns: ap.Spec.AuthScheme.Patterns, + Conditions: ap.Spec.AuthScheme.Conditions, + Identity: ap.Spec.AuthScheme.Identity, + Metadata: ap.Spec.AuthScheme.Metadata, + Authorization: ap.Spec.AuthScheme.Authorization, + Response: ap.Spec.AuthScheme.Response, + DenyWith: ap.Spec.AuthScheme.DenyWith, + }, + }, nil +} + +func (r *AuthPolicyReconciler) policyHosts(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) ([]string, error) { + if len(ap.Spec.AuthRules) == 0 { + return r.TargetHostnames(ctx, targetNetworkObject) + } + + uniqueHostnamesMap := make(map[string]interface{}) + for idx := range ap.Spec.AuthRules { + if len(ap.Spec.AuthRules[idx].Hosts) == 0 { + // When one of the rules does not have hosts, just return target hostnames + return r.TargetHostnames(ctx, targetNetworkObject) + } + + for _, hostname := range ap.Spec.AuthRules[idx].Hosts { + uniqueHostnamesMap[hostname] = nil + } + } + + hostnames := make([]string, 0, len(uniqueHostnamesMap)) + for k := range uniqueHostnamesMap { + hostnames = append(hostnames, k) + } + + return hostnames, nil +} + +// authConfigName returns the name of Authorino AuthConfig CR. +func authConfigName(apKey client.ObjectKey) string { + return fmt.Sprintf("ap-%s-%s", apKey.Namespace, apKey.Name) +} + +func alwaysUpdateAuthConfig(existingObj, desiredObj client.Object) (bool, error) { + existing, ok := existingObj.(*authorinoapi.AuthConfig) + if !ok { + return false, fmt.Errorf("%T is not an *authorinoapi.AuthConfig", existingObj) + } + desired, ok := desiredObj.(*authorinoapi.AuthConfig) + if !ok { + return false, fmt.Errorf("%T is not an *authorinoapi.AuthConfig", desiredObj) + } + + if reflect.DeepEqual(existing.Spec, desired.Spec) && reflect.DeepEqual(existing.Annotations, desired.Annotations) { + return false, nil + } + + existing.Spec = desired.Spec + existing.Annotations = desired.Annotations + return true, nil +} diff --git a/controllers/authpolicy_controller.go b/controllers/authpolicy_controller.go index f25b2e6dd..0e37ba089 100644 --- a/controllers/authpolicy_controller.go +++ b/controllers/authpolicy_controller.go @@ -2,15 +2,11 @@ package controllers import ( "context" + "encoding/json" "fmt" "github.com/go-logr/logr" - authorinov1beta1 "github.com/kuadrant/authorino/api/v1beta1" - secv1beta1types "istio.io/api/security/v1beta1" - "istio.io/api/type/v1beta1" - secv1beta1resources "istio.io/client-go/pkg/apis/security/v1beta1" apierrors "k8s.io/apimachinery/pkg/api/errors" - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" @@ -18,20 +14,18 @@ import ( "sigs.k8s.io/controller-runtime/pkg/source" gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" - kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" + api "github.com/kuadrant/kuadrant-operator/api/v1beta1" "github.com/kuadrant/kuadrant-operator/pkg/common" "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" ) +const authPolicyFinalizer = "authpolicy.kuadrant.io/finalizer" + // AuthPolicyReconciler reconciles a AuthPolicy object type AuthPolicyReconciler struct { reconcilers.TargetRefReconciler } -const authPolicyFinalizer = "authpolicy.kuadrant.io/finalizer" - -var AuthProvider = common.FetchEnv("AUTH_PROVIDER", "kuadrant-authorization") - //+kubebuilder:rbac:groups=kuadrant.io,resources=authpolicies,verbs=get;list;watch;create;update;patch;delete //+kubebuilder:rbac:groups=kuadrant.io,resources=authpolicies/finalizers,verbs=update //+kubebuilder:rbac:groups=kuadrant.io,resources=authpolicies/status,verbs=get;update;patch @@ -43,8 +37,9 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ logger.Info("Reconciling AuthPolicy") ctx := logr.NewContext(eventCtx, logger) - var ap kuadrantv1beta1.AuthPolicy - if err := r.Client().Get(ctx, req.NamespacedName, &ap); err != nil { + // fetch the authpolicy + ap := &api.AuthPolicy{} + if err := r.Client().Get(ctx, req.NamespacedName, ap); err != nil { if apierrors.IsNotFound(err) { logger.Info("no AuthPolicy found") return ctrl.Result{}, nil @@ -53,66 +48,63 @@ 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 logger.V(1).Enabled() { + jsonData, err := json.MarshalIndent(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 - } + logger.V(1).Info(string(jsonData)) } - 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 - } + markedForDeletion := ap.GetDeletionTimestamp() != nil - if err := r.removeAuthSchemes(ctx, &ap, kuadrantNamespace); err != nil { + // fetch the target network object + targetNetworkObject, err := r.FetchValidTargetRef(ctx, ap.GetTargetRef(), ap.Namespace) + if err != nil { + if !markedForDeletion { + if apierrors.IsNotFound(err) { + logger.V(1).Info("Network object not found. Cleaning up") + delResErr := r.deleteResources(ctx, ap, nil) + if delResErr == nil { + delResErr = err + } + return r.reconcileStatus(ctx, ap, delResErr) + } return ctrl.Result{}, err } + targetNetworkObject = nil // we need the object set to nil when there's an error, otherwise deleting the resources (when marked for deletion) will panic + } - if err := r.deleteNetworkResourceBackReference(ctx, &ap); err != nil { - return ctrl.Result{}, err - } + // handle authpolicy marked for deletion + if markedForDeletion { + if controllerutil.ContainsFinalizer(ap, authPolicyFinalizer) { + logger.V(1).Info("Handling removal of authpolicy object") - controllerutil.RemoveFinalizer(&ap, authPolicyFinalizer) - if err := r.UpdateResource(ctx, &ap); client.IgnoreNotFound(err) != nil { - return ctrl.Result{}, err + if err := r.deleteResources(ctx, ap, targetNetworkObject); err != nil { + return ctrl.Result{}, err + } + + logger.Info("removing finalizer") + if err := r.RemoveFinalizer(ctx, ap, authPolicyFinalizer); err != nil { + return ctrl.Result{}, err + } } - return ctrl.Result{}, nil - } - // Ignore deleted resources, this can happen when foregroundDeletion is enabled - // https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion - if ap.GetDeletionTimestamp() != nil { return ctrl.Result{}, nil } - if !controllerutil.ContainsFinalizer(&ap, authPolicyFinalizer) { - controllerutil.AddFinalizer(&ap, authPolicyFinalizer) - if err := r.UpdateResource(ctx, &ap); client.IgnoreNotFound(err) != nil { + // add finalizer to the authpolicy + if !controllerutil.ContainsFinalizer(ap, authPolicyFinalizer) { + if err := r.AddFinalizer(ctx, ap, authPolicyFinalizer); client.IgnoreNotFound(err) != nil { return ctrl.Result{Requeue: true}, err } } - specResult, specErr := r.reconcileSpec(ctx, &ap, kuadrantNamespace) - if specErr == nil && specResult.Requeue { - logger.V(1).Info("Reconciling spec not finished. Requeueing.") - return specResult, nil - } + // reconcile the authpolicy spec + specErr := r.reconcileResources(ctx, ap, targetNetworkObject) - statusResult, statusErr := r.reconcileStatus(ctx, &ap, kuadrantNamespace, specErr) + // reconcile authpolicy status + statusResult, statusErr := r.reconcileStatus(ctx, ap, specErr) if specErr != nil { return ctrl.Result{}, specErr @@ -127,304 +119,112 @@ func (r *AuthPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl.Requ return statusResult, nil } - logger.Info("successfully reconciling AuthPolicy") + logger.Info("AuthPolicy reconciled successfully") return ctrl.Result{}, nil } -func (r *AuthPolicyReconciler) reconcileSpec(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) (ctrl.Result, error) { +func (r *AuthPolicyReconciler) reconcileResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { + // validate if err := ap.Validate(); err != nil { - return ctrl.Result{}, err - } - - if err := r.enforceHierarchicalConstraints(ctx, ap); err != nil { - return ctrl.Result{}, err - } - - if err := r.reconcileNetworkResourceBackReference(ctx, ap); err != nil { - return ctrl.Result{}, err - } - - if err := r.reconcileIstioAuthorizationPolicies(ctx, ap); err != nil { - return ctrl.Result{}, err + return err } - if err := r.reconcileAuthSchemes(ctx, ap, kuadrantNamespace); err != nil { - return ctrl.Result{}, err + if err := r.validateHierarchicalRules(ctx, ap, targetNetworkObject); err != nil { + return err } - return ctrl.Result{}, nil -} - -func (r *AuthPolicyReconciler) enforceHierarchicalConstraints(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error { - targetHostnames, err := r.TargetHostnames(ctx, ap.Spec.TargetRef, ap.Namespace) + // reconcile based on gateway diffs + gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{}) if err != nil { return err } - ruleHosts := make([]string, 0) - for _, rule := range ap.Spec.AuthRules { - ruleHosts = append(ruleHosts, rule.Hosts...) - } - - if valid, invalidHost := common.ValidSubdomains(targetHostnames, ruleHosts); !valid { - return fmt.Errorf("rule host (%s) does not follow any hierarchical constraints", invalidHost) - } - - return nil -} - -func (r *AuthPolicyReconciler) reconcileAuthSchemes(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) error { - logger, err := logr.FromContext(ctx) - if err != nil { + if err := r.reconcileIstioAuthorizationPolicies(ctx, ap, targetNetworkObject, gatewayDiffObj); err != nil { return err } - authConfig, err := r.desiredAuthConfig(ctx, ap, kuadrantNamespace) - if err != nil { + if err := r.reconcileAuthConfigs(ctx, ap, targetNetworkObject); err != nil { return err } - err = r.ReconcileResource(ctx, &authorinov1beta1.AuthConfig{}, authConfig, alwaysUpdateAuthConfig) - if err != nil && !apierrors.IsAlreadyExists(err) { - logger.Error(err, "ReconcileResource failed to create/update AuthConfig resource") + // set direct back ref - i.e. claim the target network object as taken asap + if err := r.reconcileNetworkResourceDirectBackReference(ctx, ap, targetNetworkObject); err != nil { return err } - return nil + + // set annotation of policies afftecting the gateway - should be the last step, only when all the reconciliation steps succeed + return r.ReconcileGatewayPolicyReferences(ctx, ap, gatewayDiffObj) } -// reconcileIstioAuthorizationPolicies translates and reconciles `AuthRules` into an Istio AuthorizationPoilcy containing them. -func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error { - logger, err := logr.FromContext(ctx) +func (r *AuthPolicyReconciler) deleteResources(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { + // delete based on gateway diffs + gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, ap, targetNetworkObject, &common.KuadrantAuthPolicyRefsConfig{}) if err != nil { return err } - targetHostnames, err := r.TargetHostnames(ctx, ap.Spec.TargetRef, ap.Namespace) - if err != nil { + if err := r.deleteIstioAuthorizationPolicies(ctx, ap, gatewayDiffObj); err != nil { return err } - toRules := []*secv1beta1types.Rule_To{} - for _, rule := range ap.Spec.AuthRules { - hosts := rule.Hosts - if len(rule.Hosts) == 0 { - hosts = targetHostnames - } - toRules = append(toRules, &secv1beta1types.Rule_To{ - Operation: &secv1beta1types.Operation{ - Hosts: hosts, - Methods: rule.Methods, - Paths: rule.Paths, - }, - }) - } - - if len(toRules) == 0 { - targetObj, err := r.FetchValidTargetRef(ctx, ap.Spec.TargetRef, ap.Namespace) - if err != nil { - return err - } - switch route := targetObj.(type) { - case *gatewayapiv1alpha2.HTTPRoute: - // rules not set and targeting a HTTPRoute - // Compile rules from the route - httpRouterules := common.RulesFromHTTPRoute(route) - for idx := range httpRouterules { - var tmp []string - toRules = append(toRules, &secv1beta1types.Rule_To{ - Operation: &secv1beta1types.Operation{ - // copy slice - Hosts: append(tmp, httpRouterules[idx].Hosts...), - Methods: append(tmp, httpRouterules[idx].Methods...), - Paths: append(tmp, httpRouterules[idx].Paths...), - }, - }) - } - } - } - - gwKeys, err := r.TargetedGatewayKeys(ctx, ap.Spec.TargetRef, ap.Namespace) - if err != nil { + if err := r.deleteAuthConfigs(ctx, ap); err != nil { return err } - targetObjectKind := "Gateway" - if common.IsTargetRefHTTPRoute(ap.Spec.TargetRef) { - targetObjectKind = "HTTPRoute" - } - - for _, gwKey := range gwKeys { - authPolicy := secv1beta1resources.AuthorizationPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: getAuthPolicyName(gwKey.Name, string(ap.Spec.TargetRef.Name), targetObjectKind), - Namespace: gwKey.Namespace, - }, - Spec: secv1beta1types.AuthorizationPolicy{ - Action: secv1beta1types.AuthorizationPolicy_CUSTOM, - Rules: []*secv1beta1types.Rule{ - { - To: toRules, - }, - }, - Selector: &v1beta1.WorkloadSelector{ - MatchLabels: map[string]string{}, // TODO(rahulanand16nov): fetch from gateway - }, - ActionDetail: &secv1beta1types.AuthorizationPolicy_Provider{ - Provider: &secv1beta1types.AuthorizationPolicy_ExtensionProvider{ - Name: AuthProvider, - }, - }, - }, - } - - err := r.ReconcileResource(ctx, &secv1beta1resources.AuthorizationPolicy{}, &authPolicy, alwaysUpdateAuthPolicy) - if err != nil && !apierrors.IsAlreadyExists(err) { - logger.Error(err, "ReconcileResource failed to create/update AuthorizationPolicy resource") + // remove direct back ref + if targetNetworkObject != nil { + if err := r.deleteNetworkResourceDirectBackReference(ctx, ap, targetNetworkObject); err != nil { return err } } - return nil + // update annotation of policies afftecting the gateway + return r.ReconcileGatewayPolicyReferences(ctx, ap, gatewayDiffObj) } -func (r *AuthPolicyReconciler) reconcileNetworkResourceBackReference(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error { - return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(ap), ap.Spec.TargetRef, - ap.Namespace, common.AuthPolicyBackRefAnnotation) -} - -func (r *AuthPolicyReconciler) removeAuthSchemes(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string) error { - logger, err := logr.FromContext(ctx) - if err != nil { - return err - } - - logger.Info("Removing Authorino's AuthConfigs") - - apKey := client.ObjectKeyFromObject(ap) - authConfig := &authorinov1beta1.AuthConfig{ - TypeMeta: metav1.TypeMeta{ - Kind: "AuthConfig", - APIVersion: authorinov1beta1.GroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: authConfigName(apKey), - Namespace: kuadrantNamespace, - }, - } - - err = r.DeleteResource(ctx, authConfig) +func (r *AuthPolicyReconciler) validateHierarchicalRules(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { + targetHostnames, err := r.TargetHostnames(ctx, targetNetworkObject) if err != nil { - if apierrors.IsNotFound(err) { - return nil - } - logger.Error(err, "failed to delete Authorino's AuthConfig") return err } - return nil -} - -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 - } - return &authorinov1beta1.AuthConfig{ - TypeMeta: metav1.TypeMeta{ - Kind: "AuthConfig", - APIVersion: authorinov1beta1.GroupVersion.String(), - }, - ObjectMeta: metav1.ObjectMeta{ - Name: authConfigName(client.ObjectKeyFromObject(ap)), - Namespace: kuadrantNamespace, - }, - Spec: authorinov1beta1.AuthConfigSpec{ - Hosts: hosts, - Patterns: ap.Spec.AuthScheme.Patterns, - Conditions: ap.Spec.AuthScheme.Conditions, - Identity: ap.Spec.AuthScheme.Identity, - Metadata: ap.Spec.AuthScheme.Metadata, - Authorization: ap.Spec.AuthScheme.Authorization, - Response: ap.Spec.AuthScheme.Response, - DenyWith: ap.Spec.AuthScheme.DenyWith, - }, - }, nil -} - -func (r *AuthPolicyReconciler) policyHosts(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) ([]string, error) { - if len(ap.Spec.AuthRules) == 0 { - return r.TargetHostnames(ctx, ap.Spec.TargetRef, ap.Namespace) - } - - uniqueHostnamesMap := make(map[string]interface{}) - for idx := range ap.Spec.AuthRules { - if len(ap.Spec.AuthRules[idx].Hosts) == 0 { - // When one of the rules does not have hosts, just return target hostnames - return r.TargetHostnames(ctx, ap.Spec.TargetRef, ap.Namespace) - } - - for _, hostname := range ap.Spec.AuthRules[idx].Hosts { - uniqueHostnamesMap[hostname] = nil - } + ruleHosts := make([]string, 0) + for _, rule := range ap.Spec.AuthRules { + ruleHosts = append(ruleHosts, rule.Hosts...) } - hostnames := make([]string, 0, len(uniqueHostnamesMap)) - for k := range uniqueHostnamesMap { - hostnames = append(hostnames, k) + if valid, invalidHost := common.ValidSubdomains(targetHostnames, ruleHosts); !valid { + return fmt.Errorf("rule host (%s) does not follow any hierarchical constraints", invalidHost) } - return hostnames, nil + return nil } -func (r *AuthPolicyReconciler) removeIstioAuthPolicy(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error { - logger, _ := logr.FromContext(ctx) - logger.Info("Removing Istio's AuthorizationPolicy") - - gwKeys, err := r.TargetedGatewayKeys(ctx, ap.Spec.TargetRef, ap.Namespace) - if err != nil { - return nil - } - - targetObjectKind := "Gateway" - if common.IsTargetRefHTTPRoute(ap.Spec.TargetRef) { - targetObjectKind = "HTTPRoute" - } - - for _, gwKey := range gwKeys { - istioAuthPolicy := &secv1beta1resources.AuthorizationPolicy{ - ObjectMeta: metav1.ObjectMeta{ - Name: getAuthPolicyName(gwKey.Name, string(ap.Spec.TargetRef.Name), targetObjectKind), - Namespace: gwKey.Namespace, - }, - } - - if err := r.DeleteResource(ctx, istioAuthPolicy); err != nil && !apierrors.IsNotFound(err) { - logger.Error(err, "failed to delete Istio's AuthorizationPolicy") - return err - } - } - - logger.Info("removed Istio's AuthorizationPolicy") - return nil +// Ensures only one RLP targets the network resource +func (r *AuthPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { + return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(ap), targetNetworkObject, common.AuthPolicyBackRefAnnotation) } -func (r *AuthPolicyReconciler) deleteNetworkResourceBackReference(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy) error { - return r.DeleteTargetBackReference(ctx, client.ObjectKeyFromObject(ap), ap.Spec.TargetRef, - ap.Namespace, common.AuthPolicyBackRefAnnotation) +func (r *AuthPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object) error { + return r.DeleteTargetBackReference(ctx, client.ObjectKeyFromObject(ap), targetNetworkObject, common.AuthPolicyBackRefAnnotation) } // SetupWithManager sets up the controller with the Manager. func (r *AuthPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { - HTTPRouteEventMapper := &HTTPRouteEventMapper{ - Logger: r.Logger().WithName("httpRouteHandler"), + httpRouteEventMapper := &HTTPRouteEventMapper{ + Logger: r.Logger().WithName("httpRouteEventMapper"), + } + gatewayEventMapper := &GatewayEventMapper{ + Logger: r.Logger().WithName("gatewayEventMapper"), } + return ctrl.NewControllerManagedBy(mgr). - For(&kuadrantv1beta1.AuthPolicy{}). + For(&api.AuthPolicy{}). Watches( &source.Kind{Type: &gatewayapiv1alpha2.HTTPRoute{}}, - handler.EnqueueRequestsFromMapFunc(HTTPRouteEventMapper.MapToAuthPolicy), + handler.EnqueueRequestsFromMapFunc(httpRouteEventMapper.MapToAuthPolicy), ). Watches(&source.Kind{Type: &gatewayapiv1alpha2.Gateway{}}, - handler.EnqueueRequestsFromMapFunc(HTTPRouteEventMapper.MapToAuthPolicy)). + handler.EnqueueRequestsFromMapFunc(gatewayEventMapper.MapToAuthPolicy)). Complete(r) } diff --git a/controllers/authpolicy_controller_test.go b/controllers/authpolicy_controller_test.go index d342b7632..a198ff759 100644 --- a/controllers/authpolicy_controller_test.go +++ b/controllers/authpolicy_controller_test.go @@ -73,7 +73,7 @@ var _ = Describe("AuthPolicy controller", func() { name = CustomGatewayName } iapKey := types.NamespacedName{ - Name: getAuthPolicyName(name, string(authpolicies[idx].Spec.TargetRef.Name), string(authpolicies[idx].Spec.TargetRef.Kind)), + Name: istioAuthorizationPolicyName(name, authpolicies[idx].Spec.TargetRef), Namespace: namespace, } Eventually(func() bool { @@ -116,7 +116,7 @@ var _ = Describe("AuthPolicy controller", func() { name = CustomGatewayName } iapKey := types.NamespacedName{ - Name: getAuthPolicyName(name, string(authpolicies[idx].Spec.TargetRef.Name), string(authpolicies[idx].Spec.TargetRef.Kind)), + Name: istioAuthorizationPolicyName(name, authpolicies[idx].Spec.TargetRef), Namespace: namespace, } Eventually(func() bool { @@ -213,10 +213,18 @@ var _ = Describe("AuthPolicy controller", func() { }) It("Istio's authorizationpolicy should include network resource hostnames on kuadrant rules without hosts", func() { + typedNamespace := v1alpha2.Namespace(testNamespace) + targetRef := v1alpha2.PolicyTargetReference{ + Group: gatewayapiv1alpha2.Group(gatewayapiv1alpha2.GroupVersion.Group), + Kind: "HTTPRoute", + Name: gatewayapiv1alpha2.ObjectName(routeName), + Namespace: &typedNamespace, + } + // Check Istio's authorization policy rules existingIAP := &secv1beta1resources.AuthorizationPolicy{} key := types.NamespacedName{ - Name: getAuthPolicyName(gwName, routeName, "HTTPRoute"), + Name: istioAuthorizationPolicyName(gwName, targetRef), Namespace: testNamespace, } diff --git a/controllers/authpolicy_istio_authorization_policy.go b/controllers/authpolicy_istio_authorization_policy.go new file mode 100644 index 000000000..26b927a5a --- /dev/null +++ b/controllers/authpolicy_istio_authorization_policy.go @@ -0,0 +1,215 @@ +package controllers + +import ( + "context" + "fmt" + "reflect" + + "github.com/go-logr/logr" + apierrors "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/labels" + "sigs.k8s.io/controller-runtime/pkg/client" + gatewayapiv1alpha2 "sigs.k8s.io/gateway-api/apis/v1alpha2" + + istiosecurity "istio.io/api/security/v1beta1" + istiocommon "istio.io/api/type/v1beta1" + istio "istio.io/client-go/pkg/apis/security/v1beta1" + + api "github.com/kuadrant/kuadrant-operator/api/v1beta1" + "github.com/kuadrant/kuadrant-operator/pkg/common" + "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" +) + +var KuadrantExtAuthProviderName = common.FetchEnv("AUTH_PROVIDER", "kuadrant-authorization") + +// reconcileIstioAuthorizationPolicies translates and reconciles `AuthRules` into an Istio AuthorizationPoilcy containing them. +func (r *AuthPolicyReconciler) reconcileIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, targetNetworkObject client.Object, gwDiffObj *reconcilers.GatewayDiff) error { + if err := r.deleteIstioAuthorizationPolicies(ctx, ap, gwDiffObj); err != nil { + return err + } + + logger, err := logr.FromContext(ctx) + if err != nil { + return err + } + + targetHostnames, err := r.TargetHostnames(ctx, targetNetworkObject) + if err != nil { + return err + } + + // TODO(guicassolato): should the rules filter only the hostnames valid for each gateway? + toRules := istioAuthorizationPolicyRules(ap.Spec.AuthRules, targetHostnames, targetNetworkObject) + + // Create IstioAuthorizationPolicy for each gateway directly or indirectly referred by the policy (existing and new) + for _, gw := range append(gwDiffObj.GatewaysWithValidPolicyRef, gwDiffObj.GatewaysMissingPolicyRef...) { + iap := istioAuthorizationPolicy(gw.Gateway, ap, toRules) + err := r.ReconcileResource(ctx, &istio.AuthorizationPolicy{}, iap, alwaysUpdateAuthPolicy) + if err != nil && !apierrors.IsAlreadyExists(err) { + logger.Error(err, "failed to reconcile IstioAuthorizationPolicy resource") + return err + } + } + + return nil +} + +// deleteIstioAuthorizationPolicies deletes IstioAuthorizationPolicies previously created for gateways no longer targeted by the policy (directly or indirectly) +func (r *AuthPolicyReconciler) deleteIstioAuthorizationPolicies(ctx context.Context, ap *api.AuthPolicy, gwDiffObj *reconcilers.GatewayDiff) error { + logger, err := logr.FromContext(ctx) + if err != nil { + return err + } + + for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { + listOptions := &client.ListOptions{LabelSelector: labels.SelectorFromSet(labels.Set(istioAuthorizationPolicyLabels(client.ObjectKeyFromObject(gw.Gateway), client.ObjectKeyFromObject(ap))))} + iapList := &istio.AuthorizationPolicyList{} + if err := r.Client().List(ctx, iapList, listOptions); err != nil { + return err + } + + for _, iap := range iapList.Items { + // it's OK to just go ahead and delete because we only create one IAP per target network object, + // and a network object can be target by no more than one AuthPolicy + if err := r.DeleteResource(ctx, iap); err != nil && !apierrors.IsNotFound(err) { + logger.Error(err, "failed to delete IstioAuthorizationPolicy") + return err + } + } + } + + return nil +} + +func istioAuthorizationPolicy(gateway *gatewayapiv1alpha2.Gateway, ap *api.AuthPolicy, toRules []*istiosecurity.Rule_To) *istio.AuthorizationPolicy { + return &istio.AuthorizationPolicy{ + ObjectMeta: metav1.ObjectMeta{ + Name: istioAuthorizationPolicyName(gateway.Name, ap.GetTargetRef()), + Namespace: gateway.Namespace, + Labels: istioAuthorizationPolicyLabels(client.ObjectKeyFromObject(gateway), client.ObjectKeyFromObject(ap)), + }, + Spec: istiosecurity.AuthorizationPolicy{ + Action: istiosecurity.AuthorizationPolicy_CUSTOM, + Rules: []*istiosecurity.Rule{ + { + To: toRules, + }, + }, + Selector: &istiocommon.WorkloadSelector{ + MatchLabels: gateway.Labels, // FIXME: https://github.com/Kuadrant/kuadrant-operator/issues/141 + }, + ActionDetail: &istiosecurity.AuthorizationPolicy_Provider{ + Provider: &istiosecurity.AuthorizationPolicy_ExtensionProvider{ + Name: KuadrantExtAuthProviderName, + }, + }, + }, + } +} + +// istioAuthorizationPolicyName generates the name of an AuthorizationPolicy. +func istioAuthorizationPolicyName(gwName string, targetRef gatewayapiv1alpha2.PolicyTargetReference) string { + switch targetRef.Kind { + case "Gateway": + return fmt.Sprintf("on-%s", gwName) // Without this, IAP will be named: on--using-; + case "HTTPRoute": + return fmt.Sprintf("on-%s-using-%s", gwName, targetRef.Name) + } + return "" +} + +func istioAuthorizationPolicyLabels(gwKey, apKey client.ObjectKey) map[string]string { + return map[string]string{ + common.AuthPolicyBackRefAnnotation: apKey.Name, + fmt.Sprintf("%s-namespace", common.AuthPolicyBackRefAnnotation): apKey.Namespace, + "gateway-namespace": gwKey.Namespace, + "gateway": gwKey.Name, + } +} + +func istioAuthorizationPolicyRules(authRules []api.AuthRule, targetHostnames []string, targetNetworkObject client.Object) []*istiosecurity.Rule_To { + toRules := []*istiosecurity.Rule_To{} + + // Rules set in the AuthPolicy + for _, rule := range authRules { + hosts := rule.Hosts + if len(rule.Hosts) == 0 { + hosts = targetHostnames + } + toRules = append(toRules, &istiosecurity.Rule_To{ + Operation: &istiosecurity.Operation{ + Hosts: hosts, + Methods: rule.Methods, + Paths: rule.Paths, + }, + }) + } + + // TODO(guicassolato): always inherit the rules from the target network object and remove AuthRules from the AuthPolicy API + + if len(toRules) == 0 { + // Rules not set in the AuthPolicy - inherit the rules from the target network object + switch obj := targetNetworkObject.(type) { + case *gatewayapiv1alpha2.HTTPRoute: + // Rules not set and targeting a HTTPRoute - inherit the rules (hostnames, methods and paths) from the HTTPRoute + httpRouterules := common.RulesFromHTTPRoute(obj) + for idx := range httpRouterules { + toRules = append(toRules, &istiosecurity.Rule_To{ + Operation: &istiosecurity.Operation{ + Hosts: common.SliceCopy(httpRouterules[idx].Hosts), + Methods: common.SliceCopy(httpRouterules[idx].Methods), + Paths: common.SliceCopy(httpRouterules[idx].Paths), + }, + }) + } + case *gatewayapiv1alpha2.Gateway: + // Rules not set and targeting a Gateway - inherit the rules (hostnames) from the Gateway + toRules = append(toRules, &istiosecurity.Rule_To{ + Operation: &istiosecurity.Operation{ + Hosts: targetHostnames, + }, + }) + } + } + + return toRules +} + +func alwaysUpdateAuthPolicy(existingObj, desiredObj client.Object) (bool, error) { + existing, ok := existingObj.(*istio.AuthorizationPolicy) + if !ok { + return false, fmt.Errorf("%T is not an *istio.AuthorizationPolicy", existingObj) + } + desired, ok := desiredObj.(*istio.AuthorizationPolicy) + if !ok { + return false, fmt.Errorf("%T is not an *istio.AuthorizationPolicy", desiredObj) + } + + if reflect.DeepEqual(existing.Spec.Action, desired.Spec.Action) { + return false, nil + } + existing.Spec.Action = desired.Spec.Action + + if reflect.DeepEqual(existing.Spec.ActionDetail, desired.Spec.ActionDetail) { + return false, nil + } + existing.Spec.ActionDetail = desired.Spec.ActionDetail + + if reflect.DeepEqual(existing.Spec.Rules, desired.Spec.Rules) { + return false, nil + } + existing.Spec.Rules = desired.Spec.Rules + + if reflect.DeepEqual(existing.Spec.Selector, desired.Spec.Selector) { + return false, nil + } + existing.Spec.Selector = desired.Spec.Selector + + if reflect.DeepEqual(existing.Annotations, desired.Annotations) { + return false, nil + } + existing.Annotations = desired.Annotations + + return true, nil +} diff --git a/controllers/authpolicy_status.go b/controllers/authpolicy_status.go index 84e5ed02c..4cb4cc69c 100644 --- a/controllers/authpolicy_status.go +++ b/controllers/authpolicy_status.go @@ -18,7 +18,7 @@ import ( const APAvailableConditionType string = "Available" // reconcileStatus makes sure status block of AuthPolicy is up-to-date. -func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, kuadrantNamespace string, specErr error) (ctrl.Result, error) { +func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *kuadrantv1beta1.AuthPolicy, specErr error) (ctrl.Result, error) { logger, _ := logr.FromContext(ctx) logger.V(1).Info("Reconciling AuthPolicy status", "spec error", specErr) @@ -27,7 +27,7 @@ func (r *AuthPolicyReconciler) reconcileStatus(ctx context.Context, ap *kuadrant if specErr == nil { // skip fetching authconfig if we already have a reconciliation error. apKey := client.ObjectKeyFromObject(ap) authConfigKey := client.ObjectKey{ - Namespace: kuadrantNamespace, + Namespace: ap.Namespace, Name: authConfigName(apKey), } authConfig := &authorinov1beta1.AuthConfig{} @@ -77,21 +77,21 @@ func (r *AuthPolicyReconciler) calculateStatus(ap *kuadrantv1beta1.AuthPolicy, s ObservedGeneration: ap.Status.ObservedGeneration, } - targetObjectKind := string(ap.Spec.TargetRef.Kind) - availableCond := r.availableCondition(targetObjectKind, specErr, authConfigReady) + targetNetworkObjectectKind := string(ap.Spec.TargetRef.Kind) + availableCond := r.availableCondition(targetNetworkObjectectKind, specErr, authConfigReady) meta.SetStatusCondition(&newStatus.Conditions, *availableCond) return newStatus } -func (r *AuthPolicyReconciler) availableCondition(targetObjectKind string, specErr error, authConfigReady bool) *metav1.Condition { +func (r *AuthPolicyReconciler) availableCondition(targetNetworkObjectectKind string, specErr error, authConfigReady bool) *metav1.Condition { // Condition if there is not issue cond := &metav1.Condition{ Type: APAvailableConditionType, Status: metav1.ConditionTrue, - Reason: fmt.Sprintf("%sProtected", targetObjectKind), - Message: fmt.Sprintf("%s is protected", targetObjectKind), + Reason: fmt.Sprintf("%sProtected", targetNetworkObjectectKind), + Message: fmt.Sprintf("%s is protected", targetNetworkObjectectKind), } if specErr != nil { diff --git a/controllers/authpolicy_utils.go b/controllers/authpolicy_utils.go deleted file mode 100644 index 1d9ba897c..000000000 --- a/controllers/authpolicy_utils.go +++ /dev/null @@ -1,56 +0,0 @@ -package controllers - -import ( - "fmt" - - authorinov1beta1 "github.com/kuadrant/authorino/api/v1beta1" - istiosecurityv1beta1 "istio.io/client-go/pkg/apis/security/v1beta1" - - "sigs.k8s.io/controller-runtime/pkg/client" -) - -// authConfigName returns the name of Authorino AuthConfig CR. -func authConfigName(objKey client.ObjectKey) string { - return fmt.Sprintf("ap-%s-%s", objKey.Namespace, objKey.Name) -} - -// getAuthPolicyName generates the name of an AuthorizationPolicy. -func getAuthPolicyName(gwName, networkName, networkKind string) string { - if networkKind == "Gateway" { - return fmt.Sprintf("on-%s", gwName) // Without this, IAP will be named: on--using-; - } - return fmt.Sprintf("on-%s-using-%s", gwName, networkName) -} - -func alwaysUpdateAuthPolicy(existingObj, desiredObj client.Object) (bool, error) { - existing, ok := existingObj.(*istiosecurityv1beta1.AuthorizationPolicy) - if !ok { - return false, fmt.Errorf("%T is not a *istiosecurityv1beta1.AuthorizationPolicy", existingObj) - } - desired, ok := desiredObj.(*istiosecurityv1beta1.AuthorizationPolicy) - if !ok { - return false, fmt.Errorf("%T is not a *istiosecurityv1beta1.AuthorizationPolicy", desiredObj) - } - - existing.Spec.Action = desired.Spec.Action - existing.Spec.ActionDetail = desired.Spec.ActionDetail - existing.Spec.Rules = desired.Spec.Rules - existing.Spec.Selector = desired.Spec.Selector - existing.Annotations = desired.Annotations - return true, nil -} - -func alwaysUpdateAuthConfig(existingObj, desiredObj client.Object) (bool, error) { - existing, ok := existingObj.(*authorinov1beta1.AuthConfig) - if !ok { - return false, fmt.Errorf("%T is not a *authorinov1beta1.AuthConfig", existingObj) - } - desired, ok := desiredObj.(*authorinov1beta1.AuthConfig) - if !ok { - return false, fmt.Errorf("%T is not a *authorinov1beta1.AuthConfig", desiredObj) - } - - existing.Spec = desired.Spec - existing.Annotations = desired.Annotations - return true, nil -} diff --git a/controllers/gateway_eventmapper.go b/controllers/gateway_eventmapper.go index 4e54729f3..73f3ca006 100644 --- a/controllers/gateway_eventmapper.go +++ b/controllers/gateway_eventmapper.go @@ -16,22 +16,30 @@ type GatewayEventMapper struct { Logger logr.Logger } -func (h *GatewayEventMapper) MapToRateLimitPolicy(obj client.Object) []reconcile.Request { +func (m *GatewayEventMapper) MapToRateLimitPolicy(obj client.Object) []reconcile.Request { + return m.mapToPolicyRequest(obj, "ratelimitpolicy", &common.KuadrantRateLimitPolicyRefsConfig{}) +} + +func (m *GatewayEventMapper) MapToAuthPolicy(obj client.Object) []reconcile.Request { + return m.mapToPolicyRequest(obj, "authpolicy", &common.KuadrantAuthPolicyRefsConfig{}) +} + +func (m *GatewayEventMapper) mapToPolicyRequest(obj client.Object, policyKind string, policyRefsConfig common.PolicyRefsConfig) []reconcile.Request { + logger := m.Logger.V(1).WithValues("object", client.ObjectKeyFromObject(obj)) + gateway, ok := obj.(*gatewayapiv1alpha2.Gateway) if !ok { - h.Logger.V(1).Info("MapToRateLimitPolicy: gateway not received", "error", fmt.Sprintf("%T is not a *gatewayapiv1alpha2.Gateway", obj)) + logger.Info("mapToPolicyRequest:", "error", fmt.Sprintf("%T is not a *gatewayapiv1alpha2.Gateway", obj)) return []reconcile.Request{} } - gw := common.GatewayWrapper{Gateway: gateway, PolicyRefsConfig: &common.KuadrantRateLimitPolicyRefsConfig{}} + gw := common.GatewayWrapper{Gateway: gateway, PolicyRefsConfig: policyRefsConfig} requests := make([]reconcile.Request, 0) - for _, rlpKey := range gw.PolicyRefs() { - h.Logger.V(1).Info("MapToRateLimitPolicy", "ratelimitpolicy", rlpKey) - requests = append(requests, reconcile.Request{ - NamespacedName: rlpKey, - }) + for _, policyKey := range gw.PolicyRefs() { + logger.Info("mapToPolicyRequest", policyKind, policyKey) + requests = append(requests, reconcile.Request{NamespacedName: policyKey}) } return requests diff --git a/controllers/httproute_eventmapper.go b/controllers/httproute_eventmapper.go index f069f8418..279938e06 100644 --- a/controllers/httproute_eventmapper.go +++ b/controllers/httproute_eventmapper.go @@ -13,50 +13,23 @@ type HTTPRouteEventMapper struct { Logger logr.Logger } -func (h *HTTPRouteEventMapper) MapToRateLimitPolicy(obj client.Object) []reconcile.Request { - httpRouteAnnotations := obj.GetAnnotations() - if httpRouteAnnotations == nil { - httpRouteAnnotations = map[string]string{} - } - - rateLimitRef, ok := httpRouteAnnotations[common.RateLimitPolicyBackRefAnnotation] - if !ok { - return []reconcile.Request{} - } - - rlpKey := common.NamespacedNameToObjectKey(rateLimitRef, obj.GetNamespace()) - - h.Logger.V(1).Info("Processing object", "key", client.ObjectKeyFromObject(obj), "ratelimitpolicy", rlpKey) - - requests := []reconcile.Request{ - { - NamespacedName: rlpKey, - }, - } - - return requests +func (m *HTTPRouteEventMapper) MapToRateLimitPolicy(obj client.Object) []reconcile.Request { + return m.mapToPolicyRequest(obj, "ratelimitpolicy", common.RateLimitPolicyBackRefAnnotation) } -func (h *HTTPRouteEventMapper) MapToAuthPolicy(obj client.Object) []reconcile.Request { - httpRouteAnnotations := obj.GetAnnotations() - if httpRouteAnnotations == nil { - httpRouteAnnotations = map[string]string{} - } +func (m *HTTPRouteEventMapper) MapToAuthPolicy(obj client.Object) []reconcile.Request { + return m.mapToPolicyRequest(obj, "authpolicy", common.AuthPolicyBackRefAnnotation) +} - apRef, present := httpRouteAnnotations[common.AuthPolicyBackRefAnnotation] - if !present { +func (m *HTTPRouteEventMapper) mapToPolicyRequest(obj client.Object, policyKind, policyBackRefAnnotationName string) []reconcile.Request { + policyRef, found := common.ReadAnnotationsFromObject(obj)[policyBackRefAnnotationName] + if !found { return []reconcile.Request{} } - apKey := common.NamespacedNameToObjectKey(apRef, obj.GetNamespace()) - - h.Logger.V(1).Info("Processing object", "key", client.ObjectKeyFromObject(obj), "AuthPolicy", apKey) + policyKey := common.NamespacedNameToObjectKey(policyRef, obj.GetNamespace()) - requests := []reconcile.Request{ - { - NamespacedName: apKey, - }, - } + m.Logger.V(1).Info("Processing object", "object", client.ObjectKeyFromObject(obj), policyKind, policyKey) - return requests + return []reconcile.Request{{NamespacedName: policyKey}} } diff --git a/controllers/ratelimitpolicy_cluster_envoy_filter.go b/controllers/ratelimitpolicy_cluster_envoy_filter.go index 4dd40d991..16e477933 100644 --- a/controllers/ratelimitpolicy_cluster_envoy_filter.go +++ b/controllers/ratelimitpolicy_cluster_envoy_filter.go @@ -24,7 +24,7 @@ func (r *RateLimitPolicyReconciler) reconcileRateLimitingClusterEnvoyFilter(ctx logger, _ := logr.FromContext(ctx) for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: left gateways", "gw key", gw.Key()) + logger.V(1).Info("reconcileRateLimitingClusterEnvoyFilter: gateway with invalid policy ref", "gw key", gw.Key()) rlpRefs := gw.PolicyRefs() rlpKey := client.ObjectKeyFromObject(rlp) // Remove the RLP key from the reference list. Only if it exists (it should) @@ -46,7 +46,7 @@ func (r *RateLimitPolicyReconciler) reconcileRateLimitingClusterEnvoyFilter(ctx // Nothing to do for the gwDiffObj.GatewaysWithValidPolicyRef for _, gw := range gwDiffObj.GatewaysMissingPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: new gateways", "gw key", gw.Key()) + logger.V(1).Info("reconcileRateLimitingClusterEnvoyFilter: gateway missing policy ref", "gw key", gw.Key()) rlpRefs := gw.PolicyRefs() rlpKey := client.ObjectKeyFromObject(rlp) // Add the RLP key to the reference list. Only if it does not exist (it should not) @@ -88,7 +88,7 @@ func (r *RateLimitPolicyReconciler) gatewayRateLimitingClusterEnvoyFilter( }, Spec: istioapinetworkingv1alpha3.EnvoyFilter{ WorkloadSelector: &istioapinetworkingv1alpha3.WorkloadSelector{ - Labels: gw.Labels, + Labels: gw.Labels, // FIXME: https://github.com/Kuadrant/kuadrant-operator/issues/141 }, ConfigPatches: nil, }, diff --git a/controllers/ratelimitpolicy_controller.go b/controllers/ratelimitpolicy_controller.go index a37111d05..16f7af4c2 100644 --- a/controllers/ratelimitpolicy_controller.go +++ b/controllers/ratelimitpolicy_controller.go @@ -35,6 +35,8 @@ import ( "github.com/kuadrant/kuadrant-operator/pkg/reconcilers" ) +const rateLimitPolicyFinalizer = "ratelimitpolicy.kuadrant.io/finalizer" + // RateLimitPolicyReconciler reconciles a RateLimitPolicy object type RateLimitPolicyReconciler struct { reconcilers.TargetRefReconciler @@ -64,6 +66,7 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl logger.Info("Reconciling RateLimitPolicy") ctx := logr.NewContext(eventCtx, logger) + // fetch the ratelimitpolicy rlp := &kuadrantv1beta1.RateLimitPolicy{} if err := r.Client().Get(ctx, req.NamespacedName, rlp); err != nil { if apierrors.IsNotFound(err) { @@ -82,25 +85,44 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl logger.V(1).Info(string(jsonData)) } - if rlp.GetDeletionTimestamp() != nil && controllerutil.ContainsFinalizer(rlp, rateLimitPolicyFinalizer) { - if err := r.finalizeRLP(ctx, rlp); err != nil { - return ctrl.Result{}, err - } + markedForDeletion := rlp.GetDeletionTimestamp() != nil - logger.Info("removing finalizer") - controllerutil.RemoveFinalizer(rlp, rateLimitPolicyFinalizer) - if err := r.UpdateResource(ctx, rlp); client.IgnoreNotFound(err) != nil { + // fetch the target network object + targetNetworkObject, err := r.FetchValidTargetRef(ctx, rlp.GetTargetRef(), rlp.Namespace) + if err != nil { + if !markedForDeletion { + if apierrors.IsNotFound(err) { + logger.V(1).Info("Network object not found. Cleaning up") + delResErr := r.deleteResources(ctx, rlp, nil) + if delResErr == nil { + delResErr = err + } + return r.reconcileStatus(ctx, rlp, nil, delResErr) + } return ctrl.Result{}, err } - return ctrl.Result{}, nil + targetNetworkObject = nil // we need the object set to nil when there's an error, otherwise deleting the resources (when marked for deletion) will panic } - // Ignore deleted resources, this can happen when foregroundDeletion is enabled - // https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/#foreground-cascading-deletion - if rlp.GetDeletionTimestamp() != nil { + // handle authpolicy marked for deletion + if markedForDeletion { + if controllerutil.ContainsFinalizer(rlp, rateLimitPolicyFinalizer) { + logger.V(1).Info("Handling removal of ratelimitpolicy object") + + if err := r.deleteResources(ctx, rlp, targetNetworkObject); err != nil { + return ctrl.Result{}, err + } + + logger.Info("removing finalizer") + if err := r.RemoveFinalizer(ctx, rlp, rateLimitPolicyFinalizer); err != nil { + return ctrl.Result{}, err + } + } + return ctrl.Result{}, nil } + // add finalizer to the ratelimitpolicy if !controllerutil.ContainsFinalizer(rlp, rateLimitPolicyFinalizer) { controllerutil.AddFinalizer(rlp, rateLimitPolicyFinalizer) if err := r.UpdateResource(ctx, rlp); client.IgnoreNotFound(err) != nil { @@ -108,13 +130,11 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl } } - specResult, specErr := r.reconcileSpec(ctx, rlp) - if specErr == nil && specResult.Requeue { - logger.V(1).Info("Reconciling spec not finished. Requeueing.") - return specResult, nil - } + // reconcile the ratelimitpolicy spec + specErr := r.reconcileResources(ctx, rlp, targetNetworkObject) - statusResult, statusErr := r.reconcileStatus(ctx, rlp, specErr) + // reconcile ratelimitpolicy status + statusResult, statusErr := r.reconcileStatus(ctx, rlp, targetNetworkObject, specErr) if specErr != nil { return ctrl.Result{}, specErr @@ -129,57 +149,57 @@ func (r *RateLimitPolicyReconciler) Reconcile(eventCtx context.Context, req ctrl return statusResult, nil } - logger.Info("successfully reconciled RateLimitPolicy") + logger.Info("RateLimitPolicy reconciled successfully") return ctrl.Result{}, nil } -func (r *RateLimitPolicyReconciler) reconcileSpec(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy) (ctrl.Result, error) { +func (r *RateLimitPolicyReconciler) reconcileResources(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, targetNetworkObject client.Object) error { + // validate err := rlp.Validate() if err != nil { - return ctrl.Result{}, err + return err } - err = r.validateRuleHosts(ctx, rlp) + err = r.validateHierarchicalRules(ctx, rlp, targetNetworkObject) if err != nil { - return ctrl.Result{}, err + return err } - // Ensure only one RLP is targeting the Gateway/HTTPRoute - err = r.reconcileTargetBackReference(ctx, rlp) + // reconcile based on gateway diffs + gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{}) if err != nil { - return ctrl.Result{}, err + return err } - err = r.reconcileGatewayDiffs(ctx, rlp) - if err != nil { - return ctrl.Result{}, err + if err := r.reconcileLimits(ctx, rlp, gatewayDiffObj); err != nil { + return err } - return ctrl.Result{}, nil -} + if err := r.reconcileRateLimitingClusterEnvoyFilter(ctx, rlp, gatewayDiffObj); err != nil { + return err + } -func (r *RateLimitPolicyReconciler) reconcileTargetBackReference(ctx context.Context, policy common.KuadrantPolicy) error { - return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(policy), policy.GetTargetRef(), policy.GetNamespace(), common.RateLimitPolicyBackRefAnnotation) -} + if err := r.reconcileWASMPluginConf(ctx, rlp, gatewayDiffObj); err != nil { + return err + } -func (r *RateLimitPolicyReconciler) reconcileGatewayDiffs(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy) error { - // Reconcile based on gateway diffs: - // * Limits - // * WASM Plugin configuration object - // * EnvoyFilter - // * Gateway rate limit policy annotations (last) - logger, _ := logr.FromContext(ctx) + // set direct back ref - i.e. claim the target network object as taken asap + if err := r.reconcileNetworkResourceDirectBackReference(ctx, rlp, targetNetworkObject); err != nil { + return err + } + + // set annotation of policies afftecting the gateway - should be the last step, only when all the reconciliation steps succeed + return r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj) +} - gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, &common.KuadrantRateLimitPolicyRefsConfig{}) +func (r *RateLimitPolicyReconciler) deleteResources(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, targetNetworkObject client.Object) error { + // delete based on gateway diffs + gatewayDiffObj, err := r.ComputeGatewayDiffs(ctx, rlp, targetNetworkObject, &common.KuadrantRateLimitPolicyRefsConfig{}) if err != nil { return err } - if gatewayDiffObj == nil { - logger.V(1).Info("gatewayDiffObj is nil") - return nil - } - if err := r.reconcileLimits(ctx, rlp, gatewayDiffObj); err != nil { + if err := r.reconcileWASMPluginConf(ctx, rlp, gatewayDiffObj); err != nil { return err } @@ -187,20 +207,23 @@ func (r *RateLimitPolicyReconciler) reconcileGatewayDiffs(ctx context.Context, r return err } - if err := r.reconcileWASMPluginConf(ctx, rlp, gatewayDiffObj); err != nil { + if err := r.reconcileLimits(ctx, rlp, gatewayDiffObj); err != nil && !apierrors.IsNotFound(err) { return err } - // should be the last step, only when all the reconciliation steps succeed - if err := r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj); err != nil { - return err + // remove direct back ref + if targetNetworkObject != nil { + if err := r.deleteNetworkResourceDirectBackReference(ctx, rlp, targetNetworkObject); err != nil { + return err + } } - return nil + // update annotation of policies afftecting the gateway + return r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj) } -func (r *RateLimitPolicyReconciler) validateRuleHosts(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy) error { - targetHostnames, err := r.TargetHostnames(ctx, rlp.Spec.TargetRef, rlp.Namespace) +func (r *RateLimitPolicyReconciler) validateHierarchicalRules(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, targetNetworkObject client.Object) error { + targetHostnames, err := r.TargetHostnames(ctx, targetNetworkObject) if err != nil { return err } @@ -219,6 +242,15 @@ func (r *RateLimitPolicyReconciler) validateRuleHosts(ctx context.Context, rlp * return nil } +// Ensures only one RLP targets the network resource +func (r *RateLimitPolicyReconciler) reconcileNetworkResourceDirectBackReference(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object) error { + return r.ReconcileTargetBackReference(ctx, client.ObjectKeyFromObject(policy), targetNetworkObject, common.RateLimitPolicyBackRefAnnotation) +} + +func (r *RateLimitPolicyReconciler) deleteNetworkResourceDirectBackReference(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, targetNetworkObject client.Object) error { + return r.DeleteTargetBackReference(ctx, client.ObjectKeyFromObject(rlp), targetNetworkObject, common.RateLimitPolicyBackRefAnnotation) +} + // SetupWithManager sets up the controller with the Manager. func (r *RateLimitPolicyReconciler) SetupWithManager(mgr ctrl.Manager) error { httpRouteEventMapper := &HTTPRouteEventMapper{ diff --git a/controllers/ratelimitpolicy_controller_test.go b/controllers/ratelimitpolicy_controller_test.go index 4b3c455dd..476713a2c 100644 --- a/controllers/ratelimitpolicy_controller_test.go +++ b/controllers/ratelimitpolicy_controller_test.go @@ -285,7 +285,7 @@ var _ = Describe("RateLimitPolicy controller", func() { serialized, err := json.Marshal(refs) Expect(err).ToNot(HaveOccurred()) Expect(existingGateway.GetAnnotations()).To(HaveKeyWithValue( - common.KuadrantRateLimitPolicyRefAnnotation, string(serialized))) + common.RateLimitPoliciesBackRefAnnotation, string(serialized))) }) }) }) diff --git a/controllers/ratelimitpolicy_finalizers.go b/controllers/ratelimitpolicy_finalizers.go deleted file mode 100644 index 7b7e83d13..000000000 --- a/controllers/ratelimitpolicy_finalizers.go +++ /dev/null @@ -1,57 +0,0 @@ -package controllers - -import ( - "context" - - "github.com/go-logr/logr" - apierrors "k8s.io/apimachinery/pkg/api/errors" - "sigs.k8s.io/controller-runtime/pkg/client" - - kuadrantv1beta1 "github.com/kuadrant/kuadrant-operator/api/v1beta1" - "github.com/kuadrant/kuadrant-operator/pkg/common" -) - -const ( - // RateLimitPolicy finalizer - rateLimitPolicyFinalizer = "ratelimitpolicy.kuadrant.io/finalizer" -) - -func (r *RateLimitPolicyReconciler) finalizeRLP(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy) error { - logger, _ := logr.FromContext(ctx) - logger.V(1).Info("Handling removal of ratelimitpolicy object") - - gatewayDiffObj, err := r.ComputeFinalizeGatewayDiff(ctx, rlp, &common.KuadrantRateLimitPolicyRefsConfig{}) - if err != nil { - return err - } - if gatewayDiffObj == nil { - logger.V(1).Info("finalizeRLP: gatewayDiffObj is nil") - return nil - } - - if err := r.ReconcileGatewayPolicyReferences(ctx, rlp, gatewayDiffObj); err != nil { - return err - } - - if err := r.reconcileWASMPluginConf(ctx, rlp, gatewayDiffObj); err != nil { - return err - } - - if err := r.reconcileRateLimitingClusterEnvoyFilter(ctx, rlp, gatewayDiffObj); err != nil { - return err - } - - if err := r.reconcileLimits(ctx, rlp, gatewayDiffObj); err != nil && !apierrors.IsNotFound(err) { - return err - } - - if err := r.deleteTargetBackReference(ctx, rlp); err != nil { - return err - } - - return nil -} - -func (r *RateLimitPolicyReconciler) deleteTargetBackReference(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy) error { - return r.DeleteTargetBackReference(ctx, client.ObjectKeyFromObject(rlp), rlp.Spec.TargetRef, rlp.Namespace, common.RateLimitPolicyBackRefAnnotation) -} diff --git a/controllers/ratelimitpolicy_limits.go b/controllers/ratelimitpolicy_limits.go index 93f22282d..3e8a1e831 100644 --- a/controllers/ratelimitpolicy_limits.go +++ b/controllers/ratelimitpolicy_limits.go @@ -47,12 +47,12 @@ func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *ku limitIdx := rlptools.NewLimitadorIndex(limitador, logger) for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { - logger.V(1).Info("reconcileLimits: left gateways", "key", gw.Key()) + logger.V(1).Info("reconcileLimits: gateway with invalid policy ref", "key", gw.Key()) limitIdx.DeleteGateway(gw.Key()) } for _, gw := range gwDiffObj.GatewaysWithValidPolicyRef { - logger.V(1).Info("reconcileLimits: same gateways", "rlpRefs", gw.PolicyRefs()) + logger.V(1).Info("reconcileLimits: gateway with valid policy ref", "rlpRefs", gw.PolicyRefs()) gwLimits, err := r.gatewayLimits(ctx, gw, gw.PolicyRefs()) if err != nil { @@ -70,7 +70,7 @@ func (r *RateLimitPolicyReconciler) reconcileLimits(ctx context.Context, rlp *ku for _, gw := range gwDiffObj.GatewaysMissingPolicyRef { rlpRefs := append(gw.PolicyRefs(), client.ObjectKeyFromObject(rlp)) - logger.V(1).Info("reconcileLimits: new gateways", "rlpRefs", rlpRefs) + logger.V(1).Info("reconcileLimits: gateway missing policy ref", "rlpRefs", rlpRefs) gwLimits, err := r.gatewayLimits(ctx, gw, rlpRefs) if err != nil { diff --git a/controllers/ratelimitpolicy_status.go b/controllers/ratelimitpolicy_status.go index 05cb2977d..b46309500 100644 --- a/controllers/ratelimitpolicy_status.go +++ b/controllers/ratelimitpolicy_status.go @@ -21,9 +21,9 @@ const ( RLPAvailableConditionType string = "Available" ) -func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, specErr error) (ctrl.Result, error) { +func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, targetNetworkObject client.Object, specErr error) (ctrl.Result, error) { logger, _ := logr.FromContext(ctx) - newStatus, err := r.calculateStatus(ctx, rlp, specErr) + newStatus, err := r.calculateStatus(ctx, rlp, targetNetworkObject, specErr) if err != nil { return reconcile.Result{}, err } @@ -60,7 +60,7 @@ func (r *RateLimitPolicyReconciler) reconcileStatus(ctx context.Context, rlp *ku return ctrl.Result{}, nil } -func (r *RateLimitPolicyReconciler) calculateStatus(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, specErr error) (*kuadrantv1beta1.RateLimitPolicyStatus, error) { +func (r *RateLimitPolicyReconciler) calculateStatus(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy, targetNetworkObject client.Object, specErr error) (*kuadrantv1beta1.RateLimitPolicyStatus, error) { newStatus := &kuadrantv1beta1.RateLimitPolicyStatus{ // Copy initial conditions. Otherwise, status will always be updated Conditions: common.CopyConditions(rlp.Status.Conditions), @@ -69,7 +69,7 @@ func (r *RateLimitPolicyReconciler) calculateStatus(ctx context.Context, rlp *ku // Only makes sense for rlp's targeting a route if common.IsTargetRefHTTPRoute(rlp.Spec.TargetRef) { - gwRateLimits, err := r.gatewaysRateLimits(ctx, rlp) + gwRateLimits, err := r.gatewaysRateLimits(ctx, targetNetworkObject) if err != nil { return nil, err } @@ -100,22 +100,13 @@ func (r *RateLimitPolicyReconciler) availableCondition(specErr error) *metav1.Co return cond } -// gatewaysRateLimits returns all gateway level rate limits configuration from all the -// gateways where this rate limit policy adds configuration -func (r *RateLimitPolicyReconciler) gatewaysRateLimits(ctx context.Context, rlp *kuadrantv1beta1.RateLimitPolicy) ([]kuadrantv1beta1.GatewayRateLimits, error) { +// gatewaysRateLimits returns all gateway-level rate limit configurations from all the gateways targeted by the ratelimitpolicy (directly or indirectly) +func (r *RateLimitPolicyReconciler) gatewaysRateLimits(ctx context.Context, targetNetworkObject client.Object) ([]kuadrantv1beta1.GatewayRateLimits, error) { logger, _ := logr.FromContext(ctx) - gwKeys, err := r.TargetedGatewayKeys(ctx, rlp.Spec.TargetRef, rlp.Namespace) - if err != nil { - if apierrors.IsNotFound(err) { - gwKeys = make([]client.ObjectKey, 0) - } else { - return nil, err - } - } result := make([]kuadrantv1beta1.GatewayRateLimits, 0) - for _, gwKey := range gwKeys { + for _, gwKey := range r.TargetedGatewayKeys(ctx, targetNetworkObject) { gw := &gatewayapiv1alpha2.Gateway{} err := r.Client().Get(ctx, gwKey, gw) logger.V(1).Info("get gateway", "key", gwKey, "err", err) diff --git a/controllers/ratelimitpolicy_wasm_plugins.go b/controllers/ratelimitpolicy_wasm_plugins.go index 81479fb8f..10243b640 100644 --- a/controllers/ratelimitpolicy_wasm_plugins.go +++ b/controllers/ratelimitpolicy_wasm_plugins.go @@ -22,7 +22,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, logger, _ := logr.FromContext(ctx) for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: left gateways", "gw key", gw.Key()) + logger.V(1).Info("reconcileWASMPluginConf: gateway with invalid policy ref", "gw key", gw.Key()) rlpRefs := gw.PolicyRefs() rlpKey := client.ObjectKeyFromObject(rlp) // Remove the RLP key from the reference list. Only if it exists (it should) @@ -41,7 +41,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, } for _, gw := range gwDiffObj.GatewaysWithValidPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: same gateways", "gw key", gw.Key()) + logger.V(1).Info("reconcileWASMPluginConf: gateway with valid policy ref", "gw key", gw.Key()) wp, err := r.gatewayWASMPlugin(ctx, gw, gw.PolicyRefs()) if err != nil { return err @@ -53,7 +53,7 @@ func (r *RateLimitPolicyReconciler) reconcileWASMPluginConf(ctx context.Context, } for _, gw := range gwDiffObj.GatewaysMissingPolicyRef { - logger.V(1).Info("reconcileWASMPluginConf: new gateways", "gw key", gw.Key()) + logger.V(1).Info("reconcileWASMPluginConf: gateway missing policy ref", "gw key", gw.Key()) rlpRefs := gw.PolicyRefs() rlpKey := client.ObjectKeyFromObject(rlp) // Add the RLP key to the reference list. Only if it does not exist (it should not) @@ -87,7 +87,7 @@ func (r *RateLimitPolicyReconciler) gatewayWASMPlugin(ctx context.Context, gw co }, Spec: istioextensionsv1alpha1.WasmPlugin{ Selector: &istiotypev1beta1.WorkloadSelector{ - MatchLabels: gw.Labels, + MatchLabels: gw.Labels, // FIXME: https://github.com/Kuadrant/kuadrant-operator/issues/141 }, Url: rlptools.WASMFilterImageURL, PluginConfig: nil, diff --git a/pkg/common/common.go b/pkg/common/common.go index 8b5f92c8a..a83677142 100644 --- a/pkg/common/common.go +++ b/pkg/common/common.go @@ -29,14 +29,15 @@ import ( // TODO: move the const to a proper place, or get it from config const ( - KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service" - HTTPRouteKind = "HTTPRoute" - KuadrantRateLimitPolicyRefAnnotation = "kuadrant.io/ratelimitpolicies" - RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy-direct-backref" - AuthPolicyBackRefAnnotation = "kuadrant.io/authpolicy-backref" - KuadrantNamespaceLabel = "kuadrant.io/namespace" - NamespaceSeparator = '/' - LimitadorName = "limitador" + KuadrantRateLimitClusterName = "kuadrant-rate-limiting-service" + HTTPRouteKind = "HTTPRoute" + RateLimitPoliciesBackRefAnnotation = "kuadrant.io/ratelimitpolicies" + RateLimitPolicyBackRefAnnotation = "kuadrant.io/ratelimitpolicy" + AuthPoliciesBackRefAnnotation = "kuadrant.io/authpolicies" + AuthPolicyBackRefAnnotation = "kuadrant.io/authpolicy" + KuadrantNamespaceLabel = "kuadrant.io/namespace" + NamespaceSeparator = '/' + LimitadorName = "limitador" ) type KuadrantPolicy interface { @@ -88,6 +89,12 @@ func Map[T, U any](slice []T, f func(T) U) []U { return arr } +func SliceCopy[T any](s1 []T) []T { + s2 := make([]T, len(s1)) + copy(s2, s1) + return s2 +} + // MergeMapStringString Merge desired into existing. // Not Thread-Safe. Does it matter? func MergeMapStringString(existing *map[string]string, desired map[string]string) bool { diff --git a/pkg/common/gatewayapi_utils.go b/pkg/common/gatewayapi_utils.go index 6c6b9341b..795002086 100644 --- a/pkg/common/gatewayapi_utils.go +++ b/pkg/common/gatewayapi_utils.go @@ -184,10 +184,14 @@ type PolicyRefsConfig interface { type KuadrantRateLimitPolicyRefsConfig struct{} func (c *KuadrantRateLimitPolicyRefsConfig) PolicyRefsAnnotation() string { - return KuadrantRateLimitPolicyRefAnnotation + return RateLimitPoliciesBackRefAnnotation } -// TODO(guicassolato): Define KuadrantAuthPolicyRefsConfig +type KuadrantAuthPolicyRefsConfig struct{} + +func (c *KuadrantAuthPolicyRefsConfig) PolicyRefsAnnotation() string { + return AuthPoliciesBackRefAnnotation +} func GatewaysMissingPolicyRef(gwList *gatewayapiv1alpha2.GatewayList, policyKey client.ObjectKey, policyGwKeys []client.ObjectKey, config PolicyRefsConfig) []GatewayWrapper { // gateways referenced by the policy but do not have reference to it in the annotations @@ -246,10 +250,7 @@ func (g GatewayWrapper) PolicyRefs() []client.ObjectKey { return make([]client.ObjectKey, 0) } - gwAnnotations := g.GetAnnotations() - if gwAnnotations == nil { - gwAnnotations = map[string]string{} - } + gwAnnotations := ReadAnnotationsFromObject(g) val, ok := gwAnnotations[g.PolicyRefsAnnotation()] if !ok { @@ -271,10 +272,7 @@ func (g GatewayWrapper) ContainsPolicy(policyKey client.ObjectKey) bool { return false } - gwAnnotations := g.GetAnnotations() - if gwAnnotations == nil { - gwAnnotations = map[string]string{} - } + gwAnnotations := ReadAnnotationsFromObject(g) val, ok := gwAnnotations[g.PolicyRefsAnnotation()] if !ok { @@ -298,10 +296,7 @@ func (g GatewayWrapper) AddPolicy(policyKey client.ObjectKey) bool { return false } - gwAnnotations := g.GetAnnotations() - if gwAnnotations == nil { - gwAnnotations = map[string]string{} - } + gwAnnotations := ReadAnnotationsFromObject(g) val, ok := gwAnnotations[g.PolicyRefsAnnotation()] if !ok { @@ -343,10 +338,7 @@ func (g GatewayWrapper) DeletePolicy(policyKey client.ObjectKey) bool { return false } - gwAnnotations := g.GetAnnotations() - if gwAnnotations == nil { - gwAnnotations = map[string]string{} - } + gwAnnotations := ReadAnnotationsFromObject(g) val, ok := gwAnnotations[g.PolicyRefsAnnotation()] if !ok { diff --git a/pkg/common/gatewayapi_utils_test.go b/pkg/common/gatewayapi_utils_test.go index e972fca70..e759e0698 100644 --- a/pkg/common/gatewayapi_utils_test.go +++ b/pkg/common/gatewayapi_utils_test.go @@ -608,7 +608,7 @@ func TestGatewayWrapperPolicyRefsAnnotation(t *testing.T) { }, PolicyRefsConfig: &KuadrantRateLimitPolicyRefsConfig{}, } - if gw.PolicyRefsAnnotation() != KuadrantRateLimitPolicyRefAnnotation { + if gw.PolicyRefsAnnotation() != RateLimitPoliciesBackRefAnnotation { t.Fail() } } diff --git a/pkg/common/k8s_utils.go b/pkg/common/k8s_utils.go index db4ba5ecb..5d544daf2 100644 --- a/pkg/common/k8s_utils.go +++ b/pkg/common/k8s_utils.go @@ -39,6 +39,14 @@ func ObjectInfo(obj client.Object) string { return fmt.Sprintf("%s/%s", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetName()) } +func ReadAnnotationsFromObject(obj client.Object) map[string]string { + annotations := obj.GetAnnotations() + if annotations == nil { + annotations = map[string]string{} + } + return annotations +} + func TagObjectToDelete(obj client.Object) { // Add custom annotation annotations := obj.GetAnnotations() diff --git a/pkg/reconcilers/base_reconciler.go b/pkg/reconcilers/base_reconciler.go index f3d9ea1c3..2bc0de336 100644 --- a/pkg/reconcilers/base_reconciler.go +++ b/pkg/reconcilers/base_reconciler.go @@ -168,6 +168,22 @@ func (b *BaseReconciler) UpdateResourceStatus(ctx context.Context, obj client.Ob return b.Client().Status().Update(ctx, obj) } +func (b *BaseReconciler) AddFinalizer(ctx context.Context, obj client.Object, finalizer string) error { + controllerutil.AddFinalizer(obj, finalizer) + if err := b.UpdateResource(ctx, obj); client.IgnoreNotFound(err) != nil { + return err + } + return nil +} + +func (b *BaseReconciler) RemoveFinalizer(ctx context.Context, obj client.Object, finalizer string) error { + controllerutil.RemoveFinalizer(obj, finalizer) + if err := b.UpdateResource(ctx, obj); client.IgnoreNotFound(err) != nil { + return err + } + return nil +} + // SetOwnerReference sets owner as a Controller OwnerReference on owned func (b *BaseReconciler) SetOwnerReference(owner, obj client.Object) error { err := controllerutil.SetControllerReference(owner, obj, b.Scheme()) diff --git a/pkg/reconcilers/targetref_reconciler.go b/pkg/reconcilers/targetref_reconciler.go index 459606b83..e2a7d8e36 100644 --- a/pkg/reconcilers/targetref_reconciler.go +++ b/pkg/reconcilers/targetref_reconciler.go @@ -22,7 +22,6 @@ import ( "reflect" "github.com/go-logr/logr" - apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -114,92 +113,70 @@ func (r *TargetRefReconciler) FetchValidTargetRef(ctx context.Context, targetRef } // TargetedGatewayKeys returns the list of gateways that are being referenced from the target. -func (r *TargetRefReconciler) TargetedGatewayKeys(ctx context.Context, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs string) ([]client.ObjectKey, error) { - gwKeys := make([]client.ObjectKey, 0) - - if common.IsTargetRefHTTPRoute(targetRef) { - tmpNS := defaultNs - if targetRef.Namespace != nil { - tmpNS = string(*targetRef.Namespace) - } - objKey := client.ObjectKey{Name: string(targetRef.Name), Namespace: tmpNS} - httpRoute, err := r.FetchValidHTTPRoute(ctx, objKey) - if err != nil { - return nil, err - } - - for _, parentRef := range httpRoute.Spec.CommonRouteSpec.ParentRefs { - gwKey := client.ObjectKey{Name: string(parentRef.Name), Namespace: httpRoute.Namespace} +func (r *TargetRefReconciler) TargetedGatewayKeys(ctx context.Context, targetNetworkObject client.Object) []client.ObjectKey { + switch obj := targetNetworkObject.(type) { + case *gatewayapiv1alpha2.HTTPRoute: + gwKeys := make([]client.ObjectKey, 0) + for _, parentRef := range obj.Spec.CommonRouteSpec.ParentRefs { + gwKey := client.ObjectKey{Name: string(parentRef.Name), Namespace: obj.Namespace} if parentRef.Namespace != nil { gwKey.Namespace = string(*parentRef.Namespace) } gwKeys = append(gwKeys, gwKey) } - } else if common.IsTargetRefGateway(targetRef) { - gwKey := client.ObjectKey{Name: string(targetRef.Name), Namespace: defaultNs} - if targetRef.Namespace != nil { - gwKey.Namespace = string(*targetRef.Namespace) - } - gwKeys = []client.ObjectKey{gwKey} - } + return gwKeys - return gwKeys, nil -} + case *gatewayapiv1alpha2.Gateway: + return []client.ObjectKey{client.ObjectKeyFromObject(targetNetworkObject)} -func (r *TargetRefReconciler) TargetHostnames(ctx context.Context, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs string) ([]string, error) { - targetObj, err := r.FetchValidTargetRef(ctx, targetRef, defaultNs) - if err != nil { - return nil, err + // If the targetNetworkObject is nil, we don't fail; instead, we return an empty slice of gateway keys. + // This is for supporting a smooth cleanup in cases where the network object has been deleted already + default: + return []client.ObjectKey{} } +} - netResourceHosts := make([]string, 0) - switch netResource := targetObj.(type) { +func (r *TargetRefReconciler) TargetHostnames(ctx context.Context, targetNetworkObject client.Object) ([]string, error) { + hosts := make([]string, 0) + switch obj := targetNetworkObject.(type) { case *gatewayapiv1alpha2.HTTPRoute: - for _, hostname := range netResource.Spec.Hostnames { - netResourceHosts = append(netResourceHosts, string(hostname)) + for _, hostname := range obj.Spec.Hostnames { + hosts = append(hosts, string(hostname)) } case *gatewayapiv1alpha2.Gateway: - for idx := range netResource.Spec.Listeners { - if netResource.Spec.Listeners[idx].Hostname != nil { - netResourceHosts = append(netResourceHosts, string(*netResource.Spec.Listeners[idx].Hostname)) + for idx := range obj.Spec.Listeners { + if obj.Spec.Listeners[idx].Hostname != nil { + hosts = append(hosts, string(*obj.Spec.Listeners[idx].Hostname)) } } } - if len(netResourceHosts) == 0 { - netResourceHosts = append(netResourceHosts, string("*")) + if len(hosts) == 0 { + hosts = append(hosts, string("*")) } - return netResourceHosts, nil + return hosts, nil } // ReconcileTargetBackReference adds policy key in annotations of the target object -func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs, annotationName string) error { +func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetNetworkObject client.Object, annotationName string) error { logger, _ := logr.FromContext(ctx) - targetObj, err := r.FetchValidTargetRef(ctx, targetRef, defaultNs) - if err != nil { - // The object should also exist - return err - } - targetObjKey := client.ObjectKeyFromObject(targetObj) - targetObjType := targetObj.GetObjectKind().GroupVersionKind() + targetNetworkObjectKey := client.ObjectKeyFromObject(targetNetworkObject) + targetNetworkObjectKind := targetNetworkObject.GetObjectKind().GroupVersionKind() // Reconcile the back reference: - objAnnotations := targetObj.GetAnnotations() - if objAnnotations == nil { - objAnnotations = map[string]string{} - } + objAnnotations := common.ReadAnnotationsFromObject(targetNetworkObject) if val, ok := objAnnotations[annotationName]; ok { if val != policyKey.String() { - return fmt.Errorf("the %s target %s is already referenced by policy %s", targetObjType, targetObjKey, policyKey.String()) + return fmt.Errorf("the %s target %s is already referenced by policy %s", targetNetworkObjectKind, targetNetworkObjectKey, policyKey.String()) } } else { objAnnotations[annotationName] = policyKey.String() - targetObj.SetAnnotations(objAnnotations) - err := r.UpdateResource(ctx, targetObj) - logger.V(1).Info("ReconcileTargetBackReference: update target object", "type", targetObjType, "name", targetObjKey, "err", err) + targetNetworkObject.SetAnnotations(objAnnotations) + err := r.UpdateResource(ctx, targetNetworkObject) + logger.V(1).Info("ReconcileTargetBackReference: update target object", "kind", targetNetworkObjectKind, "name", targetNetworkObjectKey, "err", err) if err != nil { return err } @@ -208,31 +185,20 @@ func (r *TargetRefReconciler) ReconcileTargetBackReference(ctx context.Context, return nil } -func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetRef gatewayapiv1alpha2.PolicyTargetReference, defaultNs, annotationName string) error { +func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, policyKey client.ObjectKey, targetNetworkObject client.Object, annotationName string) error { logger, _ := logr.FromContext(ctx) - targetObj, err := r.FetchValidTargetRef(ctx, targetRef, defaultNs) - if err != nil { - if apierrors.IsNotFound(err) { - return nil - } - return err - } - - targetObjKey := client.ObjectKeyFromObject(targetObj) - targetObjType := targetObj.GetObjectKind().GroupVersionKind() + targetNetworkObjectKey := client.ObjectKeyFromObject(targetNetworkObject) + targetNetworkObjectKind := targetNetworkObject.GetObjectKind().GroupVersionKind() // Reconcile the back reference: - objAnnotations := targetObj.GetAnnotations() - if objAnnotations == nil { - objAnnotations = map[string]string{} - } + objAnnotations := common.ReadAnnotationsFromObject(targetNetworkObject) if _, ok := objAnnotations[annotationName]; ok { delete(objAnnotations, annotationName) - targetObj.SetAnnotations(objAnnotations) - err := r.UpdateResource(ctx, targetObj) - logger.V(1).Info("DeleteTargetBackReference: update network resource", "type", targetObjType, "name", targetObjKey, "err", err) + targetNetworkObject.SetAnnotations(objAnnotations) + err := r.UpdateResource(ctx, targetNetworkObject) + logger.V(1).Info("DeleteTargetBackReference: update network resource", "kind", targetNetworkObjectKind, "name", targetNetworkObjectKey, "err", err) if err != nil { return err } @@ -245,17 +211,17 @@ func (r *TargetRefReconciler) DeleteTargetBackReference(ctx context.Context, pol // * list of gateways to which the policy applies for the first time // * list of gateways to which the policy no longer applies // * list of gateways to which the policy still applies -func (r *TargetRefReconciler) ComputeGatewayDiffs(ctx context.Context, policy common.KuadrantPolicy, policyRefsConfig common.PolicyRefsConfig) (*GatewayDiff, error) { +func (r *TargetRefReconciler) ComputeGatewayDiffs(ctx context.Context, policy common.KuadrantPolicy, targetNetworkObject client.Object, policyRefsConfig common.PolicyRefsConfig) (*GatewayDiff, error) { logger, _ := logr.FromContext(ctx) - gwKeys, err := r.TargetedGatewayKeys(ctx, policy.GetTargetRef(), policy.GetNamespace()) - if err != nil { - return nil, err + var gwKeys []client.ObjectKey + if policy.GetDeletionTimestamp() == nil { + gwKeys = r.TargetedGatewayKeys(ctx, targetNetworkObject) } // TODO(rahulanand16nov): maybe think about optimizing it with a label later allGwList := &gatewayapiv1alpha2.GatewayList{} - err = r.Client().List(ctx, allGwList) + err := r.Client().List(ctx, allGwList) if err != nil { return nil, err } @@ -275,42 +241,12 @@ func (r *TargetRefReconciler) ComputeGatewayDiffs(ctx context.Context, policy co return gwDiff, nil } -func (r *TargetRefReconciler) ComputeFinalizeGatewayDiff(ctx context.Context, policy common.KuadrantPolicy, policyRefsConfig common.PolicyRefsConfig) (*GatewayDiff, error) { - logger, _ := logr.FromContext(ctx) - - // Prepare gatewayDiff object only with GatewaysWithInvalidPolicyRef list populated. - // Used for the common reconciliation methods of Limits, EnvoyFilters, WasmPlugins, etc... - gwDiff := &GatewayDiff{ - GatewaysMissingPolicyRef: nil, - GatewaysWithValidPolicyRef: nil, - GatewaysWithInvalidPolicyRef: nil, - } - - gwKeys, err := r.TargetedGatewayKeys(ctx, policy.GetTargetRef(), policy.GetNamespace()) - if err != nil && !apierrors.IsNotFound(err) { - return nil, err - } - - for _, gwKey := range gwKeys { - gw := &gatewayapiv1alpha2.Gateway{} - err := r.Client().Get(ctx, gwKey, gw) - logger.V(1).Info("ComputeFinalizeGatewayDiff", "fetch gateway", gwKey, "err", err) - if err != nil { - if apierrors.IsNotFound(err) { - continue - } - return nil, err - } - gwDiff.GatewaysWithInvalidPolicyRef = append(gwDiff.GatewaysWithInvalidPolicyRef, common.GatewayWrapper{Gateway: gw, PolicyRefsConfig: policyRefsConfig}) - } - logger.V(1).Info("ComputeFinalizeGatewayDiff", "#invalid-policy-ref", len(gwDiff.GatewaysWithInvalidPolicyRef)) - - return gwDiff, nil -} - +// ReconcileGatewayPolicyReferences updates the annotations in the Gateway resources that list to all the policies +// that directly or indirectly target the gateway, based upon a pre-computed gateway diff object func (r *TargetRefReconciler) ReconcileGatewayPolicyReferences(ctx context.Context, policy client.Object, gwDiffObj *GatewayDiff) error { logger, _ := logr.FromContext(ctx) + // delete the policy from the annotations of the gateways no longer target by the policy for _, gw := range gwDiffObj.GatewaysWithInvalidPolicyRef { if gw.DeletePolicy(client.ObjectKeyFromObject(policy)) { err := r.UpdateResource(ctx, gw.Gateway) @@ -321,6 +257,7 @@ func (r *TargetRefReconciler) ReconcileGatewayPolicyReferences(ctx context.Conte } } + // add the policy to the annotations of the gateways target by the policy for _, gw := range gwDiffObj.GatewaysMissingPolicyRef { if gw.AddPolicy(client.ObjectKeyFromObject(policy)) { err := r.UpdateResource(ctx, gw.Gateway) diff --git a/pkg/reconcilers/targetref_reconciler_test.go b/pkg/reconcilers/targetref_reconciler_test.go index 315e6eabb..169b4ba61 100644 --- a/pkg/reconcilers/targetref_reconciler_test.go +++ b/pkg/reconcilers/targetref_reconciler_test.go @@ -301,12 +301,6 @@ func TestReconcileTargetBackReference(t *testing.T) { t.Fatal(err) } - targetRef := gatewayapiv1alpha2.PolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "HTTPRoute", - Name: gatewayapiv1alpha2.ObjectName(routeName), - } - policyKey := client.ObjectKey{Name: "someName", Namespace: "someNamespace"} existingRoute := &gatewayapiv1alpha2.HTTPRoute{ @@ -359,8 +353,7 @@ func TestReconcileTargetBackReference(t *testing.T) { BaseReconciler: baseReconciler, } - err = targetRefReconciler.ReconcileTargetBackReference(ctx, policyKey, targetRef, - namespace, annotationName) + err = targetRefReconciler.ReconcileTargetBackReference(ctx, policyKey, existingRoute, annotationName) if err != nil { t.Fatal(err) } @@ -407,12 +400,6 @@ func TestTargetedGatewayKeys(t *testing.T) { t.Fatal(err) } - targetRef := gatewayapiv1alpha2.PolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "HTTPRoute", - Name: gatewayapiv1alpha2.ObjectName(routeName), - } - existingRoute := &gatewayapiv1alpha2.HTTPRoute{ TypeMeta: metav1.TypeMeta{ APIVersion: "gateway.networking.k8s.io/v1alpha2", @@ -463,19 +450,16 @@ func TestTargetedGatewayKeys(t *testing.T) { BaseReconciler: baseReconciler, } - res, err := targetRefReconciler.TargetedGatewayKeys(ctx, targetRef, namespace) - if err != nil { - t.Fatal(err) - } + keys := targetRefReconciler.TargetedGatewayKeys(ctx, existingRoute) - if len(res) != 1 { - t.Fatalf("gateway key slice length is %d and it was expected to be 1", len(res)) + if len(keys) != 1 { + t.Fatalf("gateway key slice length is %d and it was expected to be 1", len(keys)) } expectedKey := client.ObjectKey{Name: "gwName", Namespace: namespace} - if res[0] != expectedKey { - t.Fatalf("gwKey value (%+v) does not match expected (%+v)", res[0], expectedKey) + if keys[0] != expectedKey { + t.Fatalf("gwKey value (%+v) does not match expected (%+v)", keys[0], expectedKey) } } @@ -497,12 +481,6 @@ func TestTargetHostnames(t *testing.T) { t.Fatal(err) } - targetRef := gatewayapiv1alpha2.PolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "HTTPRoute", - Name: gatewayapiv1alpha2.ObjectName(routeName), - } - existingRoute := &gatewayapiv1alpha2.HTTPRoute{ TypeMeta: metav1.TypeMeta{ APIVersion: "gateway.networking.k8s.io/v1alpha2", @@ -554,7 +532,7 @@ func TestTargetHostnames(t *testing.T) { BaseReconciler: baseReconciler, } - res, err := targetRefReconciler.TargetHostnames(ctx, targetRef, namespace) + res, err := targetRefReconciler.TargetHostnames(ctx, existingRoute) if err != nil { t.Fatal(err) } @@ -589,12 +567,6 @@ func TestDeleteTargetBackReference(t *testing.T) { t.Fatal(err) } - targetRef := gatewayapiv1alpha2.PolicyTargetReference{ - Group: "gateway.networking.k8s.io", - Kind: "HTTPRoute", - Name: gatewayapiv1alpha2.ObjectName(routeName), - } - policyKey := client.ObjectKey{Name: "someName", Namespace: "someNamespace"} existingRoute := &gatewayapiv1alpha2.HTTPRoute{ @@ -650,8 +622,7 @@ func TestDeleteTargetBackReference(t *testing.T) { BaseReconciler: baseReconciler, } - err = targetRefReconciler.DeleteTargetBackReference(ctx, policyKey, targetRef, - namespace, annotationName) + err = targetRefReconciler.DeleteTargetBackReference(ctx, policyKey, existingRoute, annotationName) if err != nil { t.Fatal(err) }