[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

nstogner · 2019-12-07T03:15:07Z

What this PR does / why we need it:
Allow users to specify file content in KubeadmConfig by referencing ConfigMap and Secret objects.

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 #1846

- Add field: KubeadmConfig.spec.files[].contentFrom - Resolve ConfigMap and Secret references in kubeadm controller - Add cloudinit.File type to avoid passing misleading API-type parameters (.ContentFrom) into cloudinit generation funcs - Update BootstrapData test to parse yaml and assert on file existance - Add "sigs.k8s.io/yaml" dependency

k8s-ci-robot · 2019-12-07T03:15:11Z

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: [email protected]

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

k8s-ci-robot · 2019-12-07T03:15:15Z

Welcome @nstogner!

It looks like this is your first PR to kubernetes-sigs/cluster-api 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/cluster-api has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2019-12-07T03:15:15Z

Hi @nstogner. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

k8s-ci-robot · 2019-12-07T03:15:28Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: nstogner
To complete the pull request process, please assign chuckha
You can assign the PR to them by writing /assign @chuckha in a comment when ready.

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

Needs approval from an approver in each of these files:

OWNERS

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

nstogner · 2019-12-07T03:32:49Z

Looks like I need to get my company email associated with the CLA.

detiber · 2019-12-09T16:32:01Z

bootstrap/kubeadm/api/v1alpha3/kubeadmbootstrapconfig_types.go

+
+	// ContentFrom indicates the content for the file should be retrieved from a referenced resource. Mutually exclusive with Content.
+	// +optional
+	ContentFrom *FileContentSource `json:"contentFrom,omitempty"`


Would it make sense to move this up a level? Instead of requiring one secret or configmap per file, leverage the structure of secrets and configmaps to allow a user to import multiple files from a single configmap or secret?

That would mean we'd have files for inline, and filesFrom (?) for content from confimaps/secrets?

That would mean we'd have files for inline, and filesFrom (?) for content from confimaps/secrets?

Possibly... There is also the issue of the additional metadata that is included in the individual file types as well. It just feels quite a bit heavy weight to force one secret/configmap per file, when these resources are intended to provide multiple files by design.

Right, in light of this, I think we probably should spend some more time data modeling.

Perhaps it makes sense to copy the same pattern that pod volumes use:
https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.16/#configmapvolumesource-v1-core
Allow for specifying default file metadata (in this case owner, permissions, encoding) at the directory level (see defaultMode in the above example), and then allow for overriding those at the file level (see items[].mode in the above example).

Do the configmap & secret sources in Kubernetes offer anything like this?

Pod volumes are a slightly different use-case in that the unit of concern is a dir rather than a file. Also, the volume source and the volume mount location are specified separately.

It should be documented in the code how we are preventing two machines referencing the same ConfigMap/Secrets, because this will break the clusterctl move logic.

@fabriziopandini Noted.

Are we happy to move forward with the above data structure?

I'm ok with it.
FYI I'm trying to overcome the limitation of the single cluster on a parallel thread about clusterctl move

I'm ok moving forward. Note, we have a fairly aggressive timeline for getting v1alpha3 done, so this may make it, or be deferred to v1alpha4 - just want to be up front about that.

vincepri · 2020-03-30T16:07:38Z

going through the open PRs, I wonder what's the status of this one? Should we close it and try to spec it out in v0.4 a little more?

k8s-ci-robot · 2020-03-30T16:07:45Z

@nstogner: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

ncdc · 2020-03-30T17:28:35Z

I do think it's worth finalizing the data format before asking @nstogner to do any additional coding.

nstogner · 2020-04-07T17:35:45Z

@ncdc @vincepri I am happy to pickup work on this one again. How do you want to proceed on getting to a finalized data format?

vincepri · 2020-04-07T18:16:18Z

I'd move the discussion into a proposal google doc first, and use the CAEP template format to start the discussion and gather feedback

alexeldeib · 2020-05-10T00:32:51Z

@nstogner not sure if you're currently working on this, I started a doc and fresh PR from master incorporating these changes + my interpretation of the desired updates (added you as a git coauthor)

#1739 (comment)
#3038
https://docs.google.com/document/d/1R_E3uu8Uvfe4pOcTrfQYUolJCK8mUS58Kq4A8zmqTr8/edit?usp=sharing

vincepri · 2020-05-21T20:54:36Z

Going to close this for now given that we have other PRs soon tackling this.

/close

k8s-ci-robot · 2020-05-21T20:54:42Z

@vincepri: Closed this PR.

In response to this:

Going to close this for now given that we have other PRs soon tackling this.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

nstogner added 3 commits December 6, 2019 21:41

make generate

6e0cbdb

Remove gopkg.in/yaml.v2 which snuck into go.mod and run: go mod tidy

dff06dd

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 7, 2019

k8s-ci-robot added cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 7, 2019

k8s-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 7, 2019

k8s-ci-robot requested review from fabriziopandini and vincepri December 7, 2019 03:15

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Dec 7, 2019

detiber reviewed Dec 9, 2019

View reviewed changes

rhockenbury mentioned this pull request Jan 30, 2020

Windows Support #2218

Closed

4 tasks

ncdc added this to the Next milestone Feb 5, 2020

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Mar 30, 2020

vincepri marked this pull request as draft April 20, 2020 21:16

vincepri mentioned this pull request May 7, 2020

[capz] kubeadm bootstrap config contains raw secrets #3030

Closed

alexeldeib mentioned this pull request May 10, 2020

Document approaches for infrastructure providers to consider for securing sensitive bootstrap data #1739

Closed

alexeldeib mentioned this pull request May 11, 2020

✨ enable capbk to read secrets #3038

Closed

k8s-ci-robot closed this May 21, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

nstogner commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

nstogner commented Dec 7, 2019

detiber Dec 9, 2019

ncdc Dec 9, 2019

detiber Dec 9, 2019

ncdc Dec 9, 2019

nstogner Dec 10, 2019

nstogner Dec 18, 2019

fabriziopandini Dec 28, 2019

nstogner Jan 15, 2020

fabriziopandini Jan 15, 2020

ncdc Jan 15, 2020

vincepri commented Mar 30, 2020

k8s-ci-robot commented Mar 30, 2020

ncdc commented Mar 30, 2020

nstogner commented Apr 7, 2020

vincepri commented Apr 7, 2020

alexeldeib commented May 10, 2020 •

edited

Loading

vincepri commented May 21, 2020

k8s-ci-robot commented May 21, 2020

[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

[WIP] ✨Add support for defining KubeadmConfig files by reference (ConfigMap & Secret) #1860

Conversation

nstogner commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

k8s-ci-robot commented Dec 7, 2019

nstogner commented Dec 7, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vincepri commented Mar 30, 2020

k8s-ci-robot commented Mar 30, 2020

ncdc commented Mar 30, 2020

nstogner commented Apr 7, 2020

vincepri commented Apr 7, 2020

alexeldeib commented May 10, 2020 • edited Loading

vincepri commented May 21, 2020

k8s-ci-robot commented May 21, 2020

alexeldeib commented May 10, 2020 •

edited

Loading