FEAT: Add api-ownership properties to OperatorContext

[patriziobruno]

Description

Current implementation doesn't provide a way to manage API ownership for API definition. Being API ownership tightly tied to the user that creates/updates the API through operator, hence to OperatorContextAuth, it makes sense to have this information stored within the OperatorContext. This PR adds user_ownersanduser_group_ownerstoOperatorContext, matching secret and env vars references, and matching DashboardApi`.

Related Issue

#249

Motivation and Context

Tyk-operator allows organizations with multiple teams and one cloud instance to provision and manage APIs, but it doesn't enforce any ownership, leaving the API definitions open to accidental modification/deletion from non-owner teams.

Test Coverage For This Change

• created two SaaS Tyk users
• deployed two OperatorContext with API keys from the two users as TYK_AUTH and user ids in TYK_USER_OWNERS
• deployed an ApiDefinition for each OperatorContext
• updated the two ApiDefinition manifests swapping the OperatorContext references and got an authorization denied when trying to kubectl apply the manifests
• deleted the ApiDefinitions
• updated the OperatorContexts removing TYK_USER_OWNERS
• created the ApiDefinitions again and swapped the OperatorContext to test if the change has broken any previous behaviour, the test showed no breaking

Screenshots (if appropriate)

Types of changes

• [ ] Bug fix (non-breaking change which fixes an issue)
• [x] New feature (non-breaking change which adds functionality)
• [ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

• [x] Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If PRing from your fork, don't come from your master!
• [x] Make sure you are making a pull request against our master branch (left side). Also, it would be best if you started your change off our latest master.
• [x] Make sure you are updating CHANGELOG.md based on your changes.
• [x] My change requires a change to the documentation.
  • [ ] If you've changed APIs, describe what needs to be updated in the documentation.
• [x] I have updated the documentation accordingly.
• [x] I have added tests to cover my changes.
• [ ] All new and existing tests passed.
• [x] Check your code additions will not fail linting checks:
  • [x] gofmt -s -w .
  • [x] go vet ./...
  • [ ] golangci-lint run

[caroltyk]

@patriziobruno Thank you so much for your contribution. We'll review as soon as we can!

[asoorm]

Thank you for your contribution - I have been thinking a lot about it and am not so sure if this is the right solution. Whilst it may solve the problem for the short-term, is it scalable?

In the case of disaster recovery, or migrating between environments, and a true git-ops workflow, how do we ensure that the IDs of the users and user groups are correct? The IDs would need to be deterministic.

This means that rather than hardcoding the IDs between environments, we would need to target users and groups by name - which means potentially having User and Group CRDs. Or - we would need to find a way of ensuring that we can have explicit user_ids and group_ids irrespective of the Tyk installation.

[patriziobruno]

Isn't this true for tyk-sync as well? If I export my API definition as JSON and want to migrate it to a different environment or in case of disaster recovery, I'll need to change the ownership info before importing the JSON.

I'm not sure this is a problem that should be fixed by tyk-operator. I think API ownership needs fixing in Tyk's API, by referencing users/groups by name rather than the id. Also, for User/Group k8s resources to be references as API owners, the concept of API ownership should be embedded in the API-definition, which currently is not. API Ownership is part of the request object sent to the dashboard-api.

With this PR, the user-id is in the configuration, just like its api-credentials. If you want to publish to a different environment, you still need to change the api-credentials, what's the problem in changing the ownership info as well? Ideally, tyk-operator would set only one owner, by resolving the api-credentials' user_id. After all, this is just to prevent different operatorcontexts to modify/delete API definitions they don't own.

[patriziobruno]

hi @asoorm any thoughts on my last comment?

[asoorm]

@patriziobruno apologies for delay - i've been back2back. Will review properly this week.

[andrei-tyk]

Hi @patriziobruno

Firstly thank you for your contribution; it's really great to see the open source community come together. Secondly, after very long debates I am proud to announce that we have decided to implement your changes. The PR is undergoing testing by both me as a developer and by a colleague from the QA team. For the moment please if possible pull the latest master changes into your branch (the branch from your forked repo did not work out of the box for me but started working after I pulled the master branch). Thank you again and we'll come back here if we have any news or if we just merge the PR directly.

[patriziobruno]

Thanks, @andrei-tyk. I'm glad about this news.

[komalsukhani]

Thank you @patriziobruno for this PR. I am sure lot of customers will benefit from this. We have tested it. It looks great! This feature will be available in next release

[caroltyk]

Hi @patriziobruno Thanks for your idea and contribution again. The change is now available in v0.12.

[patriziobruno]

@komalsukhani @caroltyk @asoorm @andrei-tyk thanks, it was a pleasure working with you all