From 7bc3213e79188efdfc1eed8357fae0e0e5e1f458 Mon Sep 17 00:00:00 2001 From: Eguzki Astiz Lezaun Date: Thu, 7 Oct 2021 16:53:54 +0200 Subject: [PATCH] apiproduct: refactor api selectors --- apis/networking/v1beta1/apiproduct_types.go | 34 ++++++++++------- .../v1beta1/zz_generated.deepcopy.go | 37 ++++++------------- .../networking.kuadrant.io_apiproducts.yaml | 15 ++++---- controllers/apiproduct_controller_test.go | 12 +++--- .../istioprovider/http_route.go | 12 +++--- .../istioprovider/provider.go | 12 +++--- samples/apiproduct.yaml | 7 ++-- 7 files changed, 60 insertions(+), 69 deletions(-) diff --git a/apis/networking/v1beta1/apiproduct_types.go b/apis/networking/v1beta1/apiproduct_types.go index f76759f..9c485dd 100644 --- a/apis/networking/v1beta1/apiproduct_types.go +++ b/apis/networking/v1beta1/apiproduct_types.go @@ -56,21 +56,22 @@ type SecurityScheme struct { OpenIDConnectAuth *OpenIDConnectAuth `json:"openIDConnectAuth,omitempty"` } -type Mapping struct { - Prefix string `json:"prefix"` -} +type APIReference struct { + // Kuadrant API object name + Name string `json:"name"` -type APISelector struct { - Name string `json:"name"` + // Kuadrant API object namespace Namespace string `json:"namespace"` // +optional Tag *string `json:"tag,omitempty"` - Mapping Mapping `json:"mapping,omitempty"` + // Public prefix path to be added to all paths exposed by the API + // +optional + Prefix *string `json:"prefix,omitempty"` } -func (a *APISelector) APINamespacedName() types.NamespacedName { +func (a *APIReference) APINamespacedName() types.NamespacedName { name := a.Name if a.Tag != nil { name = APIObjectName(a.Name, *a.Tag) @@ -109,7 +110,10 @@ type APIProductSpec struct { // derived based on the underlying platform. Hosts []string `json:"hosts"` SecurityScheme []*SecurityScheme `json:"securityScheme"` - APIs []*APISelector `json:"APIs"` + + // The list of kuadrant API to be protected + // +kubebuilder:validation:MinItems=1 + APIs []APIReference `json:"APIs"` // RateLimit configures global rate limit parameters // +optional @@ -169,14 +173,18 @@ func (a *APIProduct) Validate() error { // Look for duplicated mapping prefixes mappingPrefix := map[string]interface{}{} - for idx, apiSel := range a.Spec.APIs { - apiField := apisFldPath.Index(idx) + for idx := range a.Spec.APIs { + apiRef := &a.Spec.APIs[idx] + if apiRef.Prefix == nil { + continue + } - if _, ok := mappingPrefix[apiSel.Mapping.Prefix]; ok { - fieldErrors = append(fieldErrors, field.Invalid(apiField, apiSel.APINamespacedName(), "duplicated prefix")) + apiField := apisFldPath.Index(idx) + if _, ok := mappingPrefix[*apiRef.Prefix]; ok { + fieldErrors = append(fieldErrors, field.Invalid(apiField, apiRef.APINamespacedName(), "duplicated prefix")) } - mappingPrefix[apiSel.Mapping.Prefix] = nil + mappingPrefix[*apiRef.Prefix] = nil } if len(fieldErrors) > 0 { diff --git a/apis/networking/v1beta1/zz_generated.deepcopy.go b/apis/networking/v1beta1/zz_generated.deepcopy.go index f73b542..d3b3f9b 100644 --- a/apis/networking/v1beta1/zz_generated.deepcopy.go +++ b/apis/networking/v1beta1/zz_generated.deepcopy.go @@ -228,13 +228,9 @@ func (in *APIProductSpec) DeepCopyInto(out *APIProductSpec) { } if in.APIs != nil { in, out := &in.APIs, &out.APIs - *out = make([]*APISelector, len(*in)) + *out = make([]APIReference, len(*in)) for i := range *in { - if (*in)[i] != nil { - in, out := &(*in)[i], &(*out)[i] - *out = new(APISelector) - (*in).DeepCopyInto(*out) - } + (*in)[i].DeepCopyInto(&(*out)[i]) } } if in.RateLimit != nil { @@ -277,22 +273,26 @@ func (in *APIProductStatus) DeepCopy() *APIProductStatus { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *APISelector) DeepCopyInto(out *APISelector) { +func (in *APIReference) DeepCopyInto(out *APIReference) { *out = *in if in.Tag != nil { in, out := &in.Tag, &out.Tag *out = new(string) **out = **in } - out.Mapping = in.Mapping + if in.Prefix != nil { + in, out := &in.Prefix, &out.Prefix + *out = new(string) + **out = **in + } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APISelector. -func (in *APISelector) DeepCopy() *APISelector { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new APIReference. +func (in *APIReference) DeepCopy() *APIReference { if in == nil { return nil } - out := new(APISelector) + out := new(APIReference) in.DeepCopyInto(out) return out } @@ -345,21 +345,6 @@ func (in *Destination) DeepCopy() *Destination { return out } -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Mapping) DeepCopyInto(out *Mapping) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Mapping. -func (in *Mapping) DeepCopy() *Mapping { - if in == nil { - return nil - } - out := new(Mapping) - in.DeepCopyInto(out) - return out -} - // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *OpenIDConnectAuth) DeepCopyInto(out *OpenIDConnectAuth) { *out = *in diff --git a/config/crd/bases/networking.kuadrant.io_apiproducts.yaml b/config/crd/bases/networking.kuadrant.io_apiproducts.yaml index 85c2077..581fcc0 100644 --- a/config/crd/bases/networking.kuadrant.io_apiproducts.yaml +++ b/config/crd/bases/networking.kuadrant.io_apiproducts.yaml @@ -37,18 +37,18 @@ spec: description: APIProductSpec defines the desired state of APIProduct properties: APIs: + description: The list of kuadrant API to be protected items: properties: - mapping: - properties: - prefix: - type: string - required: - - prefix - type: object name: + description: Kuadrant API object name type: string namespace: + description: Kuadrant API object namespace + type: string + prefix: + description: Public prefix path to be added to all paths exposed + by the API type: string tag: type: string @@ -56,6 +56,7 @@ spec: - name - namespace type: object + minItems: 1 type: array hosts: description: The destination hosts to which traffic is being sent. diff --git a/controllers/apiproduct_controller_test.go b/controllers/apiproduct_controller_test.go index 8e3d0c2..53b986a 100644 --- a/controllers/apiproduct_controller_test.go +++ b/controllers/apiproduct_controller_test.go @@ -182,6 +182,8 @@ var _ = Describe("APIPRoduct controller", func() { func apiProduct(ns string) *networkingv1beta1.APIProduct { tag := "production" + catsPrefix := "/cats" + dogsPrefix := "/dogs" return &networkingv1beta1.APIProduct{ TypeMeta: metav1.TypeMeta{ Kind: networkingv1beta1.APIProductKind, @@ -190,22 +192,18 @@ func apiProduct(ns string) *networkingv1beta1.APIProduct { ObjectMeta: metav1.ObjectMeta{Name: "apiproduct01", Namespace: ns}, Spec: networkingv1beta1.APIProductSpec{ Hosts: []string{"petstore.127.0.0.1.nip.io"}, - APIs: []*networkingv1beta1.APISelector{ + APIs: []networkingv1beta1.APIReference{ { Name: "cats", Namespace: ns, Tag: &tag, - Mapping: networkingv1beta1.Mapping{ - Prefix: "/cats", - }, + Prefix: &catsPrefix, }, { Name: "dogs", Namespace: ns, Tag: &tag, - Mapping: networkingv1beta1.Mapping{ - Prefix: "/dogs", - }, + Prefix: &dogsPrefix, }, }, SecurityScheme: []*networkingv1beta1.SecurityScheme{ diff --git a/pkg/ingressproviders/istioprovider/http_route.go b/pkg/ingressproviders/istioprovider/http_route.go index b26a897..3a8a654 100644 --- a/pkg/ingressproviders/istioprovider/http_route.go +++ b/pkg/ingressproviders/istioprovider/http_route.go @@ -117,7 +117,7 @@ func stringMatch(path string, matchType PathMatchType) *v1alpha3.StringMatch { } } -func HTTPRoutesFromOAS(oasContent string, pathPrefix string, destination networkingv1beta1.Destination) ([]*v1alpha3.HTTPRoute, error) { +func HTTPRoutesFromOAS(oasContent string, pathPrefix *string, destination networkingv1beta1.Destination) ([]*v1alpha3.HTTPRoute, error) { doc, err := openapi3.NewLoader().LoadFromData([]byte(oasContent)) if err != nil { return nil, err @@ -146,12 +146,12 @@ func HTTPRoutesFromOAS(oasContent string, pathPrefix string, destination network } // Handle Prefix Override. - if pathPrefix != "" { + if pathPrefix != nil { // We need to rewrite the path, to match what the service expects, basically, // removing the prefixOverride factory.RewriteURI = &path // If there's an Override, lets append it to the actual Operation Path. - factory.URIMatchPath = pathPrefix + path + factory.URIMatchPath = *pathPrefix + path } httpRoutes = append(httpRoutes, factory.HTTPRoute()) @@ -161,7 +161,7 @@ func HTTPRoutesFromOAS(oasContent string, pathPrefix string, destination network return httpRoutes, nil } -func HTTPRoutesFromPath(pathMatch *gatewayapiv1alpha1.HTTPPathMatch, pathPrefix string, destination networkingv1beta1.Destination) ([]*v1alpha3.HTTPRoute, error) { +func HTTPRoutesFromPath(pathMatch *gatewayapiv1alpha1.HTTPPathMatch, pathPrefix *string, destination networkingv1beta1.Destination) ([]*v1alpha3.HTTPRoute, error) { if pathMatch == nil { return nil, nil } @@ -186,12 +186,12 @@ func HTTPRoutesFromPath(pathMatch *gatewayapiv1alpha1.HTTPPathMatch, pathPrefix } // Handle Prefix Override. - if pathPrefix != "" { + if pathPrefix != nil { // We need to rewrite the path, to match what the service expects, basically, // removing the prefixOverride factory.RewriteURI = pathMatch.Value // If there's an Override, lets append it to the actual Operation Path. - factory.URIMatchPath = pathPrefix + *pathMatch.Value + factory.URIMatchPath = *pathPrefix + *pathMatch.Value } httpRoutes = append(httpRoutes, factory.HTTPRoute()) diff --git a/pkg/ingressproviders/istioprovider/provider.go b/pkg/ingressproviders/istioprovider/provider.go index bac77a6..3a1b1e3 100644 --- a/pkg/ingressproviders/istioprovider/provider.go +++ b/pkg/ingressproviders/istioprovider/provider.go @@ -173,8 +173,8 @@ func (is *IstioProvider) getAuthorizationPolicy(virtualService *istio.VirtualSer func (is *IstioProvider) virtualServiceFromAPIProduct(ctx context.Context, apip *networkingv1beta1.APIProduct) (*istio.VirtualService, error) { httpRoutes := []*v1alpha3.HTTPRoute{} - for _, apiSel := range apip.Spec.APIs { - apiHTTPRoutes, err := is.apiHTTPRoutes(ctx, apiSel) + for idx := range apip.Spec.APIs { + apiHTTPRoutes, err := is.apiHTTPRoutes(ctx, &apip.Spec.APIs[idx]) if err != nil { return nil, err } @@ -191,18 +191,18 @@ func (is *IstioProvider) virtualServiceFromAPIProduct(ctx context.Context, apip return factory.VirtualService(), nil } -func (is *IstioProvider) apiHTTPRoutes(ctx context.Context, apiSel *networkingv1beta1.APISelector) ([]*v1alpha3.HTTPRoute, error) { +func (is *IstioProvider) apiHTTPRoutes(ctx context.Context, apiRef *networkingv1beta1.APIReference) ([]*v1alpha3.HTTPRoute, error) { api := &networkingv1beta1.API{} - err := is.Client().Get(ctx, apiSel.APINamespacedName(), api) + err := is.Client().Get(ctx, apiRef.APINamespacedName(), api) if err != nil { return nil, err } if api.Spec.Mappings.OAS != nil { - return HTTPRoutesFromOAS(*api.Spec.Mappings.OAS, apiSel.Mapping.Prefix, api.Spec.Destination) + return HTTPRoutesFromOAS(*api.Spec.Mappings.OAS, apiRef.Prefix, api.Spec.Destination) } - return HTTPRoutesFromPath(api.Spec.Mappings.HTTPPathMatch, apiSel.Mapping.Prefix, api.Spec.Destination) + return HTTPRoutesFromPath(api.Spec.Mappings.HTTPPathMatch, apiRef.Prefix, api.Spec.Destination) } func (is *IstioProvider) Status(ctx context.Context, apip *networkingv1beta1.APIProduct) (bool, error) { diff --git a/samples/apiproduct.yaml b/samples/apiproduct.yaml index 301c02a..94c563d 100644 --- a/samples/apiproduct.yaml +++ b/samples/apiproduct.yaml @@ -1,3 +1,4 @@ +--- apiVersion: networking.kuadrant.io/v1beta1 kind: APIProduct metadata: @@ -9,13 +10,11 @@ spec: - name: dogs namespace: default tag: production - mapping: - prefix: /dogs + prefix: /dogs - name: cats namespace: default tag: production - mapping: - prefix: /cats + prefix: /cats securityScheme: - name: MyAPIKey apiKeyAuth: