tyk-operator icon indicating copy to clipboard operation
tyk-operator copied to clipboard

[TT-5089] - Attempting to remove an ApiDefinition fails if previously associated to a SecurityPolicy

Open benewen-brav opened this issue 3 years ago • 3 comments

Tyk Hybrid Gateway Version: 3.2.1 Tyk Cloud Gateway Dashboard Version: 3.2.0 Tyk Operator Version: 0.8.2

Full logs attached logs.zip

Reproduction steps:

  1. Create an ApiDefinition and SecurityPolicy that are associated
apiVersion: tyk.tyk.io/v1alpha1
kind: ApiDefinition
metadata:
  name: httpbin
spec:
  name: httpbin protected
  protocol: http
  active: true
  proxy:
    target_url: http://httpbin.org
    listen_path: /httpbin
    strip_listen_path: true
  use_standard_auth: true
  auth_configs:
    authToken:
      auth_header_name: Authorization
apiVersion: tyk.tyk.io/v1alpha1
kind: SecurityPolicy
metadata:
  name: httpbin
spec:
  id: 62505570b6e263000139f7ea
  name: Httpbin Security Policy
  state: active
  active: true
  access_rights_array:
    - name: httpbin
      namespace: default
      versions:
        - Default
  1. Remove the access_rights_array from the SecurityPolicy.
  2. Attempt to remove the ApiDefinition, but this error is returned:
{"level":"info","ts":1649434399.083711,"logger":"controllers.ApiDefinition","msg":"checking linked security policies"}
{"level":"error","ts":1649434399.0837798,"logger":"controller-runtime.manager.controller.apidefinition","msg":"Reconciler error","reconciler group":"tyk.tyk.io","reconciler kind":"ApiDefinition","name":"httpbin","namespace":"default","error":"unable to delete api due to security policy dependency=default/httpbin","stacktrace":"sigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).processNextWorkItem\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:253\nsigs.k8s.io/controller-runtime/pkg/internal/controller.(*Controller).Start.func2.2\n\t/go/pkg/mod/sigs.k8s.io/[email protected]/pkg/internal/controller/controller.go:214"}
{"level":"info","ts":1649434401.644129,"logger":"controllers.ApiDefinition","msg":"Reconciling ApiDefinition instance","ApiDefinition":"default/httpbin"}
{"level":"info","ts":1649434401.6443067,"logger":"controllers.ApiDefinition","msg":"resource being deleted"}

It appears that removing the API from the access_rights_array does not properly remove the SecurityPolicy from the linked_by_policies in the ApiDefinition:

  linked_by_policies:
    Name:       httpbin
    Namespace:  default

benewen-brav avatar Apr 08 '22 16:04 benewen-brav

Thank you for raising.

Updating SecurityPolicy.spec.access_rights_array should resolve ApiDefinition.status.linked_by_policies. At the moment, it seems to only trigger on deletion.

To recover from this problem you should be able to first delete the security policy rather than update it till issue is resolved.

asoorm avatar Apr 08 '22 19:04 asoorm

Hi @asoorm,

Thanks for the prompt reply. This would still be a problem for us as we intended to have a SecurityPolicy used for multiple APIs, so we would want to keep the SecurityPolicy around after modifying the access_rights_array. Is this expected? Or should we be creating a SecurityPolicy 1:1 with an ApiDefinition?

If you can point me to the relevant code that only works on deletion I can have a shot at a PR to fix :)

benewen-brav avatar Apr 08 '22 20:04 benewen-brav

Deletion Case: we have access to the old object because nothing actually changed - the reconciler triggers as we registered a finalizer. This means that we can clean up the API Definition status neatly.

https://github.com/TykTechnologies/tyk-operator/blob/master/controllers/securitypolicy_controller.go#L180-L182

Update Case: In order to unlink an API Definition, we will need to access the original access_rights_array prior to it being updated. I am not sure if this is even possible as it is a Kubebuilder design decision/limitation.

Thinking out loud for a potential resolution - we could potentially force Policies and the api defs that they reference to live in same namespace. We would then be able to List ApiDefinitions in current namespace and iterate through them to see if any are linked by that desired policy, but are missing from that access rights array.

A variation of the above strategy, which feels like it could be more scalable, would be to query the Tyk policy API to obtain the current state of Tyk (which we would assume was the original state prior to the update occurring). We then compare that to the desired security policy state, then any "missing" api definition references from the policy could be unlinked from it.

asoorm avatar Apr 08 '22 21:04 asoorm