Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tenant controller: store access token instead of provider key #725

Merged
merged 1 commit into from
Mar 14, 2022

Conversation

eguzki
Copy link
Member

@eguzki eguzki commented Mar 8, 2022

https://issues.redhat.com/browse/THREESCALE-7911

what

This PR changes the tenant controller to store the access token owned by the admin user of the tenant in the tenant secret referenced in spec.tenantSecretRef.

Before this PR, the tenant controller stored the tenant provider key in the tenant secret referenced in spec.tenantSecretRef. However, the provider key has permission issues for some endpoints in the 3scale Account Management API ( THREESCALE-8276 and THREESCALE-6625 and THREESCALE-7911)

The desired UX would be:

  • The customer installs 3scale and master access token is saved in system-seed
  • The customer creates a Tenant CR] with a reference to the system-seed secret
  • The customer creates Product CR and Backend CR with a reference to the tenant secret, which contains the access token.

how

Storing the access token owned by the admin user of the tenant is tricky because the tenant controller only has access to it in the response of the tenant creation 3scale API call. The access token is no longer accessible. This breaks somewhat the idempotent pattern followed by the controller. So, the implementation creates the tenant and immediately (before reconciling the admin user as it used to do), saves the access token in the tenant controller and then saves the tenant ID in the Tenant CR status field. I would like to remind without the tenant ID, there is no way to access the tenant. None the fields from the tenant is unique (like product's and backend's systemName). So storing the access token in the tenant secret and the tenant ID in the status is critical to have access to the recently created tenant. If any of the previous two steps fails, the recently created tenant will not be accessible and the controller will generate a new tenant. In that case, the previous tenant will be left unused.

verification steps

Deploy 3scale with APIManager CR

k apply -f - <<EOF
apiVersion: apps.3scale.net/v1alpha1
kind: APIManager
metadata:
  name: apimanager1
spec:
  wildcardDomain: example.com
  resourceRequirementsEnabled: false
  system:
    fileStorage:
      simpleStorageService:
        configurationSecretRef:
          name: aws-auth
EOF

Create a new tenant:

k apply -f - <<EOF
apiVersion: capabilities.3scale.net/v1alpha1
kind: Tenant
metadata:
  name: ecorp-tenant
spec:
  username: admin
  systemMasterUrl: https://master.example.com
  email: admin@example.com
  organizationName: ECorp
  masterCredentialsRef:
    name: system-seed
  passwordCredentialsRef:
    name: ecorp-admin-secret
  tenantSecretRef:
    name: ecorp-tenant-secret
EOF

The secret ecorp-tenant-secret should keep the access token and not the provider key of the new tenant as it used to be

The access token, allows to create backends and products linked to that backend.

First create the backend

k apply -f - <<EOF
---
apiVersion: capabilities.3scale.net/v1beta1
kind: Backend
metadata:
  name: backend-a
spec:
  name: "Operated Backend A"
  systemName: "backenda"
  privateBaseURL: "https://exampleA.com"
  providerAccountRef:
    name: ecorp-tenant-secret
EOF

Then create the product with references to the backend

k apply -f - <<EOF
---
apiVersion: capabilities.3scale.net/v1beta1
kind: Product
metadata:
  name: product1
spec:
  name: "OperatedProduct 1"
  backendUsages:
    backenda:
      path: /v1
  providerAccountRef:
    name: ecorp-tenant-secret
EOF

Both the backend and product resource status should show available condition

@eguzki eguzki requested review from KevFan and MStokluska March 8, 2022 09:58
@MStokluska
Copy link
Contributor

👀

@eguzki eguzki force-pushed the tenant-access-token branch from 729b1c6 to 4565e04 Compare March 11, 2022 09:11
@codeclimate
Copy link

codeclimate bot commented Mar 11, 2022

Code Climate has analyzed commit 4565e04 and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2
Duplication 2
Style 2

View more on Code Climate.

@MStokluska
Copy link
Contributor

Tested on cluster, works as expected, looking at the code now

@MStokluska
Copy link
Contributor

Tried it on cluster and it works as expected. Reviewed the code and it looks good to me.
/lgtm

@eguzki eguzki merged commit 302d3e8 into master Mar 14, 2022
@eguzki eguzki deleted the tenant-access-token branch March 14, 2022 14:39
jbride added a commit to redhat-na-ssa/keycloak_customizations_quickstart that referenced this pull request Apr 27, 2022
jbride added a commit to redhat-na-ssa/keycloak_customizations_quickstart that referenced this pull request Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants