make security requirements optional (#56)

* make security requirements optional

reconcile adding and removing sec from apiproduct

* fix apiproduct status field for optional resources

apis/networking/v1beta1/apiproduct_types.go

@@ -108,8 +108,11 @@ type APIProductSpec struct {
 	// platform, short-names can also be used instead of a FQDN (i.e. has no
 	// dots in the name). In such a scenario, the FQDN of the host would be
 	// derived based on the underlying platform.
-	Hosts          []string          `json:"hosts"`
-	SecurityScheme []*SecurityScheme `json:"securityScheme"`
+	Hosts []string `json:"hosts"`
+
+	// Configure authentication mechanisms
+	// +optional
+	SecurityScheme []SecurityScheme `json:"securityScheme,omitempty"`
 
 	// The list of kuadrant API to be protected
 	// +kubebuilder:validation:MinItems=1
@@ -238,6 +241,10 @@ func (a *APIProduct) IsPreAuthRateLimitEnabled() bool {
 		a.GlobalRateLimit() != nil
 }
 
+func (a *APIProduct) HasSecurity() bool {
+	return len(a.Spec.SecurityScheme) > 0
+}
+
 func (a *APIProduct) HasAPIKeyAuth() bool {
 	for _, securityScheme := range a.Spec.SecurityScheme {
 		if securityScheme.APIKeyAuth != nil {

config/crd/bases/networking.kuadrant.io_apiproducts.yaml

@@ -123,6 +123,7 @@ spec:
                     type: object
                 type: object
               securityScheme:
+                description: Configure authentication mechanisms
                 items:
                   properties:
                     apiKeyAuth:
@@ -161,7 +162,6 @@ spec:
             required:
             - APIs
             - hosts
-            - securityScheme
             type: object
           status:
             description: APIProductStatus defines the observed state of APIProduct

controllers/apiproduct_controller_test.go

@@ -206,7 +206,7 @@ func apiProduct(ns string) *networkingv1beta1.APIProduct {
 					Prefix:    &dogsPrefix,
 				},
 			},
-			SecurityScheme: []*networkingv1beta1.SecurityScheme{
+			SecurityScheme: []networkingv1beta1.SecurityScheme{
 				{
 					Name: "MyAPIKey",
 					APIKeyAuth: &networkingv1beta1.APIKeyAuth{

pkg/authproviders/authorino/provider.go

@@ -71,6 +71,11 @@ func buildAuthConfig(apip *networkingv1beta1.APIProduct) *authorino.AuthConfig {
 		},
 	}
 
+	if !apip.HasSecurity() {
+		common.TagObjectToDelete(authConfig)
+		return authConfig
+	}
+
 	for _, securityScheme := range apip.Spec.SecurityScheme {
 		identity := authorino.Identity{
 			Name:           securityScheme.Name,
@@ -153,17 +158,19 @@ func (a *Provider) Status(ctx context.Context, apip *networkingv1beta1.APIProduc
 	// If any object is missing/not-created, Status returns false.
 	authConfig := buildAuthConfig(apip)
 
-	existingAuthConfig := &authorino.AuthConfig{}
-	err := a.Client().Get(ctx, client.ObjectKeyFromObject(authConfig), existingAuthConfig)
-	if err != nil && errors.IsNotFound(err) {
-		return false, nil
-	} else if err != nil {
-		return false, err
+	if !common.IsObjectTaggedToDelete(authConfig) {
+		existingAuthConfig := &authorino.AuthConfig{}
+		err := a.Client().Get(ctx, client.ObjectKeyFromObject(authConfig), existingAuthConfig)
+		if err != nil && errors.IsNotFound(err) {
+			return false, nil
+		} else if err != nil {
+			return false, err
+		}
+		// TODO(jmprusi): No status in authorino serviceConfig objects, yet.
+		//if !existingServiceConfig.Status.Ready {
+		//	return false, nil
+		//}
 	}
-	// TODO(jmprusi): No status in authorino serviceConfig objects, yet.
-	//if !existingServiceConfig.Status.Ready {
-	//	return false, nil
-	//}
 
 	return true, nil
 }

pkg/ingressproviders/istioprovider/provider.go

@@ -95,7 +95,7 @@ func (is *IstioProvider) Reconcile(ctx context.Context, apip *networkingv1beta1.
 		return ctrl.Result{}, err
 	}
 
-	authPolicy, err := is.getAuthorizationPolicy(existingVS)
+	authPolicy, err := is.getAuthorizationPolicy(apip, existingVS)
 	if err != nil {
 		return ctrl.Result{}, err
 	}
@@ -121,17 +121,7 @@ func (is *IstioProvider) ReconcileIstioAuthorizationPolicy(ctx context.Context,
 	return is.ReconcileResource(ctx, &istioSecurity.AuthorizationPolicy{}, desired, mutatefn)
 }
 
-func (is *IstioProvider) getAuthorizationPolicy(virtualService *istio.VirtualService) (*istioSecurity.AuthorizationPolicy, error) {
-	policyHosts := []string{}
-
-	// Now we need to generate the list of hosts that will match the authorization policy,
-	// to be sure, we will match the "$host" and "$host:*".
-	for _, host := range virtualService.Spec.Hosts {
-		//TODO(jmprusi): avoid duplicates
-		hostSplitted := strings.Split(host, ":")
-		policyHosts = append(policyHosts, hostSplitted[0]+":*")
-		policyHosts = append(policyHosts, hostSplitted[0])
-	}
+func (is *IstioProvider) getAuthorizationPolicy(apip *networkingv1beta1.APIProduct, virtualService *istio.VirtualService) (*istioSecurity.AuthorizationPolicy, error) {
 	authPolicy := &istioSecurity.AuthorizationPolicy{
 		TypeMeta: metav1.TypeMeta{
 			Kind:       "AuthorizationPolicy",
@@ -141,26 +131,42 @@ func (is *IstioProvider) getAuthorizationPolicy(virtualService *istio.VirtualSer
 			Name:      virtualService.GetName(),
 			Namespace: common.KuadrantNamespace,
 		},
-		Spec: v1beta12.AuthorizationPolicy{
-			Selector: &v1beta13.WorkloadSelector{
-				MatchLabels: map[string]string{
-					"app": "istio-ingressgateway",
-				},
+	}
+
+	if !apip.HasSecurity() {
+		common.TagObjectToDelete(authPolicy)
+		return authPolicy, nil
+	}
+
+	// Now we need to generate the list of hosts that will match the authorization policy,
+	// to be sure, we will match the "$host" and "$host:*".
+	policyHosts := []string{}
+	for _, host := range virtualService.Spec.Hosts {
+		//TODO(jmprusi): avoid duplicates
+		hostSplitted := strings.Split(host, ":")
+		policyHosts = append(policyHosts, hostSplitted[0]+":*")
+		policyHosts = append(policyHosts, hostSplitted[0])
+	}
+
+	authPolicy.Spec = v1beta12.AuthorizationPolicy{
+		Selector: &v1beta13.WorkloadSelector{
+			MatchLabels: map[string]string{
+				"app": "istio-ingressgateway",
 			},
-			Rules: []*v1beta12.Rule{
-				{
-					To: []*v1beta12.Rule_To{{
-						Operation: &v1beta12.Operation{
-							Hosts: policyHosts,
-						},
-					}},
-				},
+		},
+		Rules: []*v1beta12.Rule{
+			{
+				To: []*v1beta12.Rule_To{{
+					Operation: &v1beta12.Operation{
+						Hosts: policyHosts,
+					},
+				}},
 			},
-			Action: v1beta12.AuthorizationPolicy_CUSTOM,
-			ActionDetail: &v1beta12.AuthorizationPolicy_Provider{
-				Provider: &v1beta12.AuthorizationPolicy_ExtensionProvider{
-					Name: common.KuadrantAuthorizationProvider,
-				},
+		},
+		Action: v1beta12.AuthorizationPolicy_CUSTOM,
+		ActionDetail: &v1beta12.AuthorizationPolicy_Provider{
+			Provider: &v1beta12.AuthorizationPolicy_ExtensionProvider{
+				Name: common.KuadrantAuthorizationProvider,
 			},
 		},
 	}

pkg/ingressproviders/istioprovider/rate_limit.go

@@ -106,7 +106,7 @@ func postAuthEnvoyFilter(apip *networkingv1beta1.APIProduct) *istionetworkingv1a
 		Patches:    make([]*istioapiv1alpha3.EnvoyFilter_EnvoyConfigObjectPatch, 0),
 	}
 
-	if apip.AuthRateLimit() == nil {
+	if apip.AuthRateLimit() == nil || !apip.HasSecurity() {
 		envoyFilter := factory.EnvoyFilter()
 		common.TagObjectToDelete(envoyFilter)
 		return envoyFilter

pkg/ratelimitproviders/limitador/provider.go

@@ -110,37 +110,40 @@ func (p *Provider) Delete(ctx context.Context, apip *networkingv1beta1.APIProduc
 func (p *Provider) Status(ctx context.Context, apip *networkingv1beta1.APIProduct) (bool, error) {
 	log := p.Logger().WithValues("apiproduct", client.ObjectKeyFromObject(apip))
 	log.V(1).Info("Status")
-	if apip.Spec.RateLimit == nil {
-		return true, nil
-	}
 
 	// Right now, we just try to get all the objects that should have been created, and check their status.
 	// If any object is missing/not-created, Status returns false.
 	desiredGlobalRateLimit := p.globalRateLimit(apip)
-	existing := &limitadorv1alpha1.RateLimit{}
-	err := p.GetResource(ctx, client.ObjectKeyFromObject(desiredGlobalRateLimit), existing)
-	if err != nil && apierrors.IsNotFound(err) {
-		return false, nil
-	} else if err != nil {
-		return false, err
+	if !common.IsObjectTaggedToDelete(desiredGlobalRateLimit) {
+		existing := &limitadorv1alpha1.RateLimit{}
+		err := p.GetResource(ctx, client.ObjectKeyFromObject(desiredGlobalRateLimit), existing)
+		if err != nil && apierrors.IsNotFound(err) {
+			return false, nil
+		} else if err != nil {
+			return false, err
+		}
 	}
 
 	desiredperRemoteIPRateLimit := p.perRemoteIPRateLimit(apip)
-	existing = &limitadorv1alpha1.RateLimit{}
-	err = p.GetResource(ctx, client.ObjectKeyFromObject(desiredperRemoteIPRateLimit), existing)
-	if err != nil && apierrors.IsNotFound(err) {
-		return false, nil
-	} else if err != nil {
-		return false, err
+	if !common.IsObjectTaggedToDelete(desiredperRemoteIPRateLimit) {
+		existing := &limitadorv1alpha1.RateLimit{}
+		err := p.GetResource(ctx, client.ObjectKeyFromObject(desiredperRemoteIPRateLimit), existing)
+		if err != nil && apierrors.IsNotFound(err) {
+			return false, nil
+		} else if err != nil {
+			return false, err
+		}
 	}
 
 	authenticatedAuthRateLimit := p.authenticatedRateLimit(apip)
-	existing = &limitadorv1alpha1.RateLimit{}
-	err = p.GetResource(ctx, client.ObjectKeyFromObject(authenticatedAuthRateLimit), existing)
-	if err != nil && apierrors.IsNotFound(err) {
-		return false, nil
-	} else if err != nil {
-		return false, err
+	if !common.IsObjectTaggedToDelete(authenticatedAuthRateLimit) {
+		existing := &limitadorv1alpha1.RateLimit{}
+		err := p.GetResource(ctx, client.ObjectKeyFromObject(authenticatedAuthRateLimit), existing)
+		if err != nil && apierrors.IsNotFound(err) {
+			return false, nil
+		} else if err != nil {
+			return false, err
+		}
 	}
 
 	return true, nil