🌱 clusterctl: support envsubst in clusterctl config #7343

sbueringer · 2022-10-05T09:40:51Z

What this PR does / why we need it:
This PR adds support for envsubst for the URL/ file path variables in the clusterctl config.

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

sbueringer · 2022-10-05T12:50:50Z

@CecileRobertMichon @enxebre @fabriziopandini @vincepri

Hey folks,
It would be really nice to get this PR merged & cherry-picked to release-1.2 as we would like to use it for the ClusterAPI KubeCon tutorial based on CAPI v1.2.3. It would really make the tutorial a lot smoother as we wouldn't have to tell folks to
customize their local clusterctl repository with their Windows/macOS/Linux file paths.

WDYT?

sbueringer · 2022-10-05T12:51:03Z

/test pull-cluster-api-e2e-full-main

sbueringer · 2022-10-05T12:52:17Z

cmd/clusterctl/client/repository/overrides.go

@@ -69,6 +70,11 @@ func (o *overrides) Path() string {
 	f, err := o.configVariablesClient.Get(overrideFolderKey)
 	if err == nil && strings.TrimSpace(f) != "" {
 		basepath = f
+
+		evaluatedBasePath, err := envsubst.Eval(basepath, os.Getenv)
+		if err == nil {


@fabriziopandini We have a few options here:

log the error

silently ignore the error

change the interface so the Path() func can return an error

What would you prefer?

Given that we are backporting, let's log the error; the we can eventually made a followup PR to change the interface in main

Makes sense, done ptal :)

sbueringer · 2022-10-05T12:55:36Z

/test pull-cluster-api-e2e-full-main

fabriziopandini

lgtm pending adding the log line in Path

Signed-off-by: Stefan Büringer [email protected]

sbueringer · 2022-10-06T13:35:01Z

/test pull-cluster-api-e2e-full-main

fabriziopandini · 2022-10-07T11:09:36Z

/lgtm
/approve

k8s-ci-robot · 2022-10-07T11:09:42Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fabriziopandini

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~~ [fabriziopandini]

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

sbueringer · 2022-10-07T13:02:05Z

/cherry-pick release-1.2

k8s-infra-cherrypick-robot · 2022-10-07T13:02:17Z

@sbueringer: cannot checkout release 1-2: error checking out release 1-2: exit status 1. output: error: pathspec 'release 1-2' did not match any file(s) known to git

In response to this:

/cherry-pick release 1-2

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-infra-cherrypick-robot · 2022-10-07T13:02:40Z

@sbueringer: #7343 failed to apply on top of branch "release-1.2":

Applying: clusterctl: support envsubst in clusterctl config
Using index info to reconstruct a base tree...
M	cmd/clusterctl/client/config/providers_client.go
M	docs/book/src/clusterctl/configuration.md
Falling back to patching base and 3-way merge...
Auto-merging docs/book/src/clusterctl/configuration.md
CONFLICT (content): Merge conflict in docs/book/src/clusterctl/configuration.md
Auto-merging cmd/clusterctl/client/config/providers_client.go
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 clusterctl: support envsubst in clusterctl config
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherry-pick release-1.2

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 added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 5, 2022

k8s-ci-robot requested review from JoelSpeed and stmcginnis October 5, 2022 09:41

sbueringer changed the title ~~[WIP] clusterctl: support relative path and HOME/CLUSTERCTL_REPO_PATH env vars~~ [WIP] 🌱 clusterctl: support relative path and HOME/CLUSTERCTL_REPO_PATH env vars Oct 5, 2022

sbueringer force-pushed the pr-clusterctl-path branch from 93e11b7 to 18b5854 Compare October 5, 2022 12:29

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 5, 2022

sbueringer changed the title ~~[WIP] 🌱 clusterctl: support relative path and HOME/CLUSTERCTL_REPO_PATH env vars~~ 🌱 clusterctl: support ${HOME} and ${CLUSTERCTL_REPOSITORY_HOME} in clusterctl config Oct 5, 2022

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 5, 2022

sbueringer force-pushed the pr-clusterctl-path branch from 18b5854 to a421772 Compare October 5, 2022 12:45

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 5, 2022

sbueringer changed the title ~~🌱 clusterctl: support ${HOME} and ${CLUSTERCTL_REPOSITORY_HOME} in clusterctl config~~ 🌱 clusterctl: support envsubst in clusterctl config Oct 5, 2022

sbueringer mentioned this pull request Oct 5, 2022

Substitute environment variables in clusterctl.yaml #5457

Closed

sbueringer commented Oct 5, 2022

View reviewed changes

sbueringer force-pushed the pr-clusterctl-path branch from a421772 to 60528f4 Compare October 5, 2022 12:55

k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 5, 2022

sbueringer mentioned this pull request Oct 5, 2022

clusterctl: Consider re-using repositoryClientFactory #7346

Closed

sbueringer force-pushed the pr-clusterctl-path branch from 60528f4 to e9f33d2 Compare October 5, 2022 16:25

sbueringer mentioned this pull request Oct 5, 2022

Tasks for Cluster API v1.2.3 #7292

Closed

5 tasks

fabriziopandini reviewed Oct 6, 2022

View reviewed changes

clusterctl: support envsubst in clusterctl config

19410e1

Signed-off-by: Stefan Büringer [email protected]

sbueringer force-pushed the pr-clusterctl-path branch from e9f33d2 to 19410e1 Compare October 6, 2022 13:34

k8s-ci-robot removed the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 6, 2022

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 6, 2022

k8s-ci-robot assigned fabriziopandini Oct 7, 2022

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 7, 2022

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 7, 2022

k8s-ci-robot merged commit 8d7585a into kubernetes-sigs:main Oct 7, 2022

k8s-ci-robot added this to the v1.3 milestone Oct 7, 2022

sbueringer deleted the pr-clusterctl-path branch October 7, 2022 11:46

This was referenced Oct 7, 2022

[release-1.2] 🌱 clusterctl: support envsubst in clusterctl config #7364

Merged

🌱 clusterctl: adjust Overrider interface so Path can return an error #7369

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🌱 clusterctl: support envsubst in clusterctl config #7343

🌱 clusterctl: support envsubst in clusterctl config #7343

sbueringer commented Oct 5, 2022 •

edited

Loading

sbueringer commented Oct 5, 2022

sbueringer commented Oct 5, 2022

sbueringer Oct 5, 2022 •

edited

Loading

fabriziopandini Oct 6, 2022

sbueringer Oct 6, 2022

sbueringer commented Oct 5, 2022

fabriziopandini left a comment

sbueringer commented Oct 6, 2022

fabriziopandini commented Oct 7, 2022

k8s-ci-robot commented Oct 7, 2022

sbueringer commented Oct 7, 2022

k8s-infra-cherrypick-robot commented Oct 7, 2022

k8s-infra-cherrypick-robot commented Oct 7, 2022

🌱 clusterctl: support envsubst in clusterctl config #7343

🌱 clusterctl: support envsubst in clusterctl config #7343

Conversation

sbueringer commented Oct 5, 2022 • edited Loading

sbueringer commented Oct 5, 2022

sbueringer commented Oct 5, 2022

sbueringer Oct 5, 2022 • edited Loading

Choose a reason for hiding this comment

fabriziopandini Oct 6, 2022

Choose a reason for hiding this comment

sbueringer Oct 6, 2022

Choose a reason for hiding this comment

sbueringer commented Oct 5, 2022

fabriziopandini left a comment

Choose a reason for hiding this comment

sbueringer commented Oct 6, 2022

fabriziopandini commented Oct 7, 2022

k8s-ci-robot commented Oct 7, 2022

sbueringer commented Oct 7, 2022

k8s-infra-cherrypick-robot commented Oct 7, 2022

k8s-infra-cherrypick-robot commented Oct 7, 2022

sbueringer commented Oct 5, 2022 •

edited

Loading

sbueringer Oct 5, 2022 •

edited

Loading