🌱 Cluster Inventory(ClusterProfile) API Provider

[everpeace]

This PR adds Cluster Inventory(ClusterProfile) API Provider.

The implementation heavily refers to the Cluster API Provider (/providers/cluster-api).

Note:

• Originally, this PR only supported Secret-based kubeconfig resolution.
• After this PR was opened, 🌱 feat: Add ClusterProfile provider with credential providers #55 was opened in parallel with this PR by @hdp617.
• We discussed and agreed to merge #55 into this PR.
• Thus,
  • #55's commit was cherry-picked into this PR in 97baa6 as agreed in #55(comment), and
  • 58bdaed unifies both implementations into the cluster-inventory-api provider

fixes #30

[corentone]

Hello @everpeace thank you for opening the PR!
Given we have the proposal of adding credentials via plugin and a PR in this repo to support kubeconfigs I wonder if we need this intersection? We may be able to wait a tiny bit and use the KEP 5339?

[everpeace]

@corentone

Given we have the proposal of adding kubernetes/enhancements#5338 and a PR in this repo to #45 I wonder if we need this intersection?

Oh, thanks for the info. I now understand kubeconfig based providers can basically support "Push Model via Credentials in Secret (Not Recommended)" in KEP-4322.

However, the kubeconfig provider seems to use different labels and key name for the Secret.

We may be able to wait a tiny bit and use the KEP 5339?

Yeah, agreed! Then, I will wait for KEP-5339 and the consumer library PR being merged and want to update this provider or add some implementation so to extract credentials via credslibrary as designed in KEP-5339 later.

WDYT??

[mjudeikis]

Looks good. One thing I got while reading this code that I dont think we handle case where cluster secret changes and we need to update-engaged cluster. @embik. do think this is the case and we need to address it?

[everpeace]

@mjudeikis

One thing I got while reading this code that I dont think we handle case where cluster secret changes and we need to update-engaged cluster.

Cool! Thanks for the catch!

f4c93f0 adds the watch for Secrets and re-engaging the Cluster in manager when kubeconfig was updated.

I'm not so confident for my implementation being appropriate or not because I'm new to multicluster-runtime, particularly, how to stop/disengage existing Cluster in the manager (I just cancel() the context for the Cluster).

I would be appreciated if you gave review and some advice.

---

manual test result is also updated: https://gist.github.com/everpeace/4ba04f70830504fb1485279ba031d907

[embik]

I'm not so confident for my implementation being appropriate or not because I'm new to multicluster-runtime, particularly, how to stop/disengage existing Cluster in the manager (I just cancel() the context for the Cluster).

That's absolutely correct!

[everpeace]

@corentone
kubeconfig-based provider seems not following the secret management specification defined in KEP-4322: Cluster Inventory - Push Model via Credentials in Secret. It uses the different labels, data key, and has no consumer concept.

So, how about this cluster-inventory-api provider keeping the support fetching kubeconfig from Secret objects managed as defined in the KEP.

And, I'd like to support KEP-5339 in another PR once the KEP and its helper library being merged.
WDYT??

@mjudeikis @embik I implemented re-engaging the Cluster in Manager when its kubeconfig was updated. And, I also unit test for the provider including re-engaging test case by envtest.

Please take a look.

[hdp617]

@corentone kubeconfig-based provider seems not following the secret management specification defined in KEP-4322: Cluster Inventory - Push Model via Credentials in Secret. It uses the different labels, data key, and has no consumer concept.

If I understand correctly from PR #45, the secret label and key in the kubeconfig provider are customizable. To work with the Cluster Inventory spec, we only need to set the KubeconfigSecretLabel and KubeconfigSecretKey options in the provider?

[everpeace]

To work with the Cluster Inventory spec, we only need to set the KubeconfigSecretLabel and KubeconfigSecretKey options in the provider?

I understand it is configurable. However, in the Cluster Inventory API KEP, one Cluster Profile can have multiple Secrets so that it can support multiple consumers like this:

kind: ClusterProfile
metadata:
  name: cluster1
..
---
kind: Secret
metadata:
  name: kubeconfig-cluster1-for-consumer-x-but-name-is-not-important
  metadata:
    labels:
      x-k8s.io/cluster-inventory-consumer: consumer-x
      x-k8s.io/cluster-profile: cluster1
data:
  Config: ...
---
kind: Secret
metadata:
  name: kubeconfig-cluster1-for-consumer-y-but-name-is-not-important
  metadata:
    labels:
      x-k8s.io/cluster-inventory-consumer: consumer-y
      x-k8s.io/cluster-profile: cluster1
data:
  Config: ...

From the user perspective, when using the Cluster Inventory API:

• The cluster name that the provider handles should be ClusterProfile's name instead of the Secret name. Of course, I understand that another level of configurability can be added to the kubeconfig-based provider, allowing another label to specify the cluster name
• It is quite natural for me that Cluster Inventory API provider support fetching kubeconfig as defined in the KEP.

[everpeace]

@embik Thank you for your review! I addressed your comments. PTAL.

[embik]

Sorry for this PR moving slowly! I haven't forgotten it, just haven't gotten around to a better understanding of ClusterProfile so I can fully review this PR. But I promise it's on my TODO list.

[everpeace]

Thank you for updating your status! I'm looking forward to your another review round👍

[everpeace]

@embik @hdp617

as agreed in #55 (comment):

• I cherry-picked @hdp617's commit in 97baa6e c5701e0
• 1ed9400 unifies both implementations into the cluster-inventory-api provider

And,

• test was also unified into mine
• examples are now based on CredentialProvider-based. (Secret-based strategy is comment-out just for reference)

PTAL 🙇

---

@corentone @qiujian16

This provider now supports both CredentialProvider-based approach(KEP-5339) and Secret-based approach(KEP-4322). PTAL.

[embik]

I think we aren't running tests successfully yet, let's try to get them working!

[embik]

/approve

Thank you so much for this contribution!

[k8s-ci-robot]

LGTM label has been added.

Git tree hash: f156a1b5d54aef87ee9bd71325ab0f63b4ce6a3b

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: embik, everpeace

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [embik]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment