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

feat: multi-tenancy #142

Merged
merged 5 commits into from
Mar 5, 2024
Merged

feat: multi-tenancy #142

merged 5 commits into from
Mar 5, 2024

Conversation

cbzzz
Copy link
Contributor

@cbzzz cbzzz commented Feb 27, 2024

What type of PR is this?
/kind feature

What this PR does / why we need it:
Adds multi-tenancy for CAPL-managed clusters. Clusters may now reference an optional Secret containing their Linode credentials (i.e. API token) to be used for the deployment of "things" associated with their cluster.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

  • From the design, it appears LinodeVPC and LinodeCluster objects are loosely-coupled and managed somewhat independently by the controller. I thought it was best to use a separate CredentialsRef field in the Spec for LinodeVPCs rather than doing a bigger refactor and trying to couple the two types.

  • The $LINODE_TOKEN environment variable is always propagated into a Secret that will be created for every workload cluster. While a credentials Secret may be referenced by multiple workload clusters (or not at all), I thought generating a per-cluster Secret was best in the current cluster templating workflow.

TODOs:

  • squashed commits
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

Testing:

  1. Create two Linode tokens, one with full "Read/Write" permissions ($GOOD_TOKEN) and one with full "None" permissions ($BAD_TOKEN).

  2. Deploy two workload clusters, one per token:

❯ LINODE_TOKEN=$GOOD_TOKEN clusterctl generate cluster ${CLUSTER_NAME}-good \
  --kubernetes-version v1.29.1 \
  --infrastructure linode:0.0.0 \
  | kubectl apply -f -

❯ LINODE_TOKEN=$BAD_TOKEN clusterctl generate cluster ${CLUSTER_NAME}-bad \
  --kubernetes-version v1.29.1 \
  --infrastructure linode:0.0.0 \
  | kubectl apply -f -
  1. A new credentials Secret is created for every workload cluster:
❯ kubectl get secrets
NAME                             TYPE                                   DATA   AGE
cbang-capl-bad-credentials       Opaque                                 1      10m
cbang-capl-good-credentials      Opaque                                 1      10m
  1. The workload cluster with the valid token will provision successfully while the one with the invalid token will fail:
❯ kubectl get clusters
NAMESPACE   NAME              CLUSTERCLASS   PHASE         AGE     VERSION
default     cbang-capl-good                  Provisioned   10m
default     cbang-capl-bad                   Failed        10m

❯ kubectl logs -n capl-system capl-controller-manager-67cdf96488-q682z
2024-03-04T14:00:00Z    DEBUG   events  [401] Your OAuth token is not authorized to use this endpoint.  {"type": "Warning", "object": {"kind":"LinodeCluster","namespace":"default","name":"cbang-capl-bad","uid":"9ec42cbe-3214-4bf7-b9be-5eadcdf6d2bd","apiVersion":"infrastructure.cluster.x-k8s.io/v1alpha1","resourceVersion":"38418"}, "reason": "CreateError"}

NOTE: These tokens will be different from the default one (capl-system/capl-manager-credentials) that is used by CAPL if the Cluster does not reference a credentials Secret.

@cbzzz cbzzz added feature New feature or request go Pull requests that update Go code labels Feb 27, 2024
@cbzzz cbzzz changed the title feat: multi-tenancy for Linode feat: multi-tenancy Feb 27, 2024
@AshleyDumaine AshleyDumaine self-requested a review February 27, 2024 16:02
@mhmxs mhmxs mentioned this pull request Mar 2, 2024
@cbzzz cbzzz force-pushed the feat.multi-tenancy branch from 60779f6 to 601ecfc Compare March 4, 2024 14:04
@cbzzz cbzzz marked this pull request as ready for review March 4, 2024 14:05
AshleyDumaine
AshleyDumaine previously approved these changes Mar 4, 2024
Copy link
Contributor

@AshleyDumaine AshleyDumaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a docs update on multi-tenancy under the topics section? 🙏

@AshleyDumaine AshleyDumaine dismissed their stale review March 4, 2024 15:16

wrong button

@cbzzz
Copy link
Contributor Author

cbzzz commented Mar 4, 2024

Can we add a docs update on multi-tenancy under the topics section? 🙏

Can do! That one in-line comment wasn't doing it for you? 🙈

cloud/scope/common.go Outdated Show resolved Hide resolved
@cbzzz cbzzz force-pushed the feat.multi-tenancy branch 2 times, most recently from ddcbc71 to 83f5d52 Compare March 4, 2024 16:14

CAPL can manage multi-tenant workload clusters across Linode accounts.
LinodeCluster objects may reference an optional Secret containing their Linode
credentials (i.e. API token) to be used for the deployment of "things"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would slightly change the wording to note that this is for Linode resources associated with individual workload clusters such as Linodes, VPCs, and NodeBalancers.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better? Worse? 🤔 9942590

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just replace "things" in the original paragraph with Linode resources (i.e. Linodes, VPCs, and NodeBalancers)

@AshleyDumaine AshleyDumaine added the documentation Improvements or additions to documentation label Mar 4, 2024
@cbzzz cbzzz force-pushed the feat.multi-tenancy branch 3 times, most recently from be1bf95 to 03a5321 Compare March 4, 2024 21:49
Adds a new (optional) field (CredentialsRef) to the Spec for LinodeClusters. This references
a Secret that contains Linode credentials that will override the ones supplied by the
controller.
Updates LinodeMachines to use the CredentialsRef field (if supplied) of their owner
LinodeCluster.
Adds a new (optional) field (CredentialsRef) to the Spec for LinodeVPCs. This references
a Secret that contains Linode credentials that will override the ones supplied by the
controller.
Adds a credentials Secret in the cluster templates. This propagates the $LINODE_TOKEN
environment variable into a Secret that will be created for every workload cluster. While this
credentials Secret may be referenced by multiple workload clusters (or not at all), generating
a per-cluster Secret works best in the current cluster templating workflow.
@cbzzz cbzzz force-pushed the feat.multi-tenancy branch from 03a5321 to 94126db Compare March 4, 2024 21:50
Copy link
Contributor

@AshleyDumaine AshleyDumaine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cbzzz cbzzz merged commit 18ff77d into main Mar 5, 2024
6 checks passed
@cbzzz cbzzz deleted the feat.multi-tenancy branch March 5, 2024 14:53
cbzzz pushed a commit that referenced this pull request Mar 7, 2024
@cbzzz cbzzz mentioned this pull request Mar 7, 2024
4 tasks
cbzzz added a commit that referenced this pull request Mar 7, 2024
amold1 pushed a commit that referenced this pull request May 17, 2024
* feat: multi-tenancy for LinodeClusters

Adds a new (optional) field (CredentialsRef) to the Spec for LinodeClusters. This references
a Secret that contains Linode credentials that will override the ones supplied by the
controller.

* feat: multi-tenancy for LinodeMachines

Updates LinodeMachines to use the CredentialsRef field (if supplied) of their owner
LinodeCluster.

* feat: multi-tenancy for LinodeVPCs

Adds a new (optional) field (CredentialsRef) to the Spec for LinodeVPCs. This references
a Secret that contains Linode credentials that will override the ones supplied by the
controller.

* templates: add cluster credentials secret

Adds a credentials Secret in the cluster templates. This propagates the $LINODE_TOKEN
environment variable into a Secret that will be created for every workload cluster. While this
credentials Secret may be referenced by multiple workload clusters (or not at all), generating
a per-cluster Secret works best in the current cluster templating workflow.

* docs: add multi-tenant clusters
amold1 pushed a commit that referenced this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature New feature or request go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants