BenWolstencroft commented 2 years ago

We're using the Tyk Operator Ingress controller to create api configurations within tyk, and also using Tyk Operator to define our policies within tyk, however, operator appears to be unable to reconcile access right entries within policies..

It appears that the only way to achieve this is to use the automatically generated full name of the API that's created within Tyk by the controller from the ingress defintion.. this is unacceptable as it's an auto-generate name that we do not know at design/dev time.

We believe that the best solution is for the reconciler to be able to find api's based on the name of the tyk ingress within k8s.

Example

API Definition (Template):

apiVersion: tyk.tyk.io/v1alpha1
kind: ApiDefinition
metadata:
  name: myapideftemplate
  labels:
    template: "true"
spec:
  name: foo
  protocol: http
  use_keyless: true
  proxy:
    target_url: http://example.com

Ingress:

apiVersion: networking.k8s.io/v1
kind: Ingress
metadata:
  name: httpbin-ingress
  annotations:
    kubernetes.io/ingress.class: tyk
    tyk.io/template: myapideftemplate # <--- refers to the api definition (template) above
spec:
    rules:
    - http:
      paths:
      - path: /httpbin
        pathType: Prefix
        backend:
          service:
            name: httpbin
            port:
              number: 8000

Security Policy:

apiVersion: tyk.tyk.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: policy-give-me-access
spec:
  name: 'Give Me Access'
  state: active
  active: true
  access_rights_array:

## Option 1:
  - name: httpbin-ingress # <-  refers to the ingress above, does not work
    namespace: default
    versions:
    - Default

## Option 2:

  - name: myapideftemplate # <-  refers to the api definition (template) above, does not work
    namespace: default
    versions:
    - Default

## Option 3:

  - name: auto-generated-ingres-full-name-1u72yba # <-  refers to the api definition generated by the controller from the ingress definition, works, but unacceptable behaviour
    namespace: default
    versions:
    - Default

  quota_max: 10
  quota_renewal_rate: 60
  rate: 5
  per: 5
  throttle_interval: 2
  throttle_retry_limit: 2

BenWolstencroft commented 2 years ago

Just spotted that this issue is the same as https://github.com/TykTechnologies/tyk-operator-internal/issues/18

caroltyk commented 2 years ago

Hi @BenWolstencroft , we're reviewing this issue now. We couldn't use the name from ApiDefinition template / Ingress as multiple APIs could be generated from them. Would it be okay if we document how the API name is generated by our controller?

BenWolstencroft commented 2 years ago

Hi @BenWolstencroft , we're reviewing this issue now. We couldn't use the name from ApiDefinition template / Ingress as multiple APIs could be generated from them. Would it be okay if we document how the API name is generated by our controller?

Apologies if my understanding is incorrect here but is it not the case that single tyk classType ingress will result in a single API in tyk? therefore if we had a method to refer to the ingresses by namespace and name within the SecurityPolicy that should only yeild a single Api defined in tyk?

If the above isn't the case then detailed understand of how names are generated would be useful if also accompanied with examples of how to refer to them across helm charts across multiple environments

In our use case we have the following:

A shared API helm chart which defines our Deployment, Service, API Definition Template and Ingress
- This chart is reused for all of our API's, and is deployed multiple times to a kubernetes cluster with configuration specific to the API we are deploying (it's image, its config etc.)
A helm chart which defines our Security Policies
- At design time we do not know the 'name' of the APIs that will ultimately end up in Tyk from the deployment of the shared API helm chart above

The problem is compounded by the fact that we deploy all of the above to a second kubernetes cluster as a persistent testing and training environment, so for each API Definition / Ingress combo, we'll get one API name in Tyk for the first cluster, and another name for our second cluster.

buraksekili commented 2 years ago

Hi @BenWolstencroft, thank you for raising this one. We, as a team, will internally investigate this issue to find a way to a solution. Until then, let me explain how API names are generated by the Ingress controller.

The Ingress controller creates Tyk APIs for each path defined under each Ingress rule. For example, consider this example: the first rule has two paths (/ and /httpbin) and the second rule has one path (/pathonly). Our Ingress controller currently creates an API for each path, where each API's name comes from the <namespace>-<ingress_name>-<short hash of (Host + Path)> (https://github.com/TykTechnologies/tyk-operator/blob/master/controllers/ingress_controller.go#L151-L152). So, for the first rule, Ingress controller generates default-httpbin-ingress-a1863f096 where

default is the namespace,
httpbin-ingress is Ingress resource name,
a1863f096 short hash (created by this function) of httpbin.ahmet/ where httpbin.ahmet is host name and / is path for this rule.

BenWolstencroft commented 2 years ago

Thank you @buraksekili

Incidentally i've done some more work on this problem and have found a workaround which stops us from having to use the generated names..

Using the Helm lookup capabilities we've been able to develop a way to targetlabelled / annotated api's within kubernetes and then have helm do the discovery work at deploy time

  {{- range $index, $apidefinition := (lookup "tyk.tyk.io/v1alpha1" "ApiDefinition" "" "").items }}
  {{- if not (hasKey $apidefinition.metadata.labels "template") }}
  {{- if (contains "public" $apidefinition.spec.domain) }}
    - name: {{ quote $apidefinition.metadata.name }}
      namespace: {{ quote $apidefinition.metadata.namespace }}
      versions:
      - Default
  {{- end }}
  {{- end }}
  {{- end }}

In the use-case in this example above, we have a policy which allows access to any 'public' api, so we filter based on the 'domain' containing the word public

This does however mean that we have to regularly run our deployment of the policy helm chart to ensure that the policies are kept up to date as API definitions are added or changed.

caroltyk commented 2 years ago

Hi @BenWolstencroft, thanks a lot for letting us know the workaround!

caroltyk commented 2 years ago

Closing the issue with workaround identified. Please track with https://github.com/TykTechnologies/tyk-operator-internal/issues/18 with future development

TykTechnologies / tyk-operator

[TT-3672] Ingress Controller + Security Policy incompatibility #485

Example