Skip to content

🐛 Quote path with -p env in envtest-setup #1513

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

Conversation

DirectXMan12
Copy link
Contributor

@DirectXMan12 DirectXMan12 commented May 5, 2021

The path could have spaces, single quotes, etc in in, so this quotes the
path and escapes single quotes so we can export properly.

This is very relevant on darwin, where our default store path contains spaces (it includes "Application Support")

The path could have spaces, single quotes, etc in in, so this quotes the
path and escapes single quotes so we can export properly.
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 5, 2021
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 5, 2021
@DirectXMan12
Copy link
Contributor Author

FYI @vincepri

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 5, 2021
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, vincepri

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 [DirectXMan12,vincepri]

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

@DirectXMan12
Copy link
Contributor Author

/retest

@DirectXMan12
Copy link
Contributor Author

hmm... looks like a flaky timing test? Occaisional timeouts waiting for etcd of kube-apiserver to start, esp on the "it should be fast to set up and tear down" tests.

/retest

// - shell strings that are next to each other are concatenated (so "a""b""c" == "abc")
// - you can intermix quote styles using the above
// - so `'"'"'` --> CLOSE_QUOTE + "'" + OPEN_QUOTE
shellQuoted := strings.ReplaceAll(path, "'", `'"'"'`)
Copy link
Contributor

Choose a reason for hiding this comment

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

So the result is supposed to look something like the following?

$ export FOO='/foo/'"'"'Application Support'"'"''
$ echo $FOO
/foo/'Application Support'

@k8s-ci-robot k8s-ci-robot merged commit 55a329c into kubernetes-sigs:master May 5, 2021
@k8s-ci-robot k8s-ci-robot added this to the v0.9.x milestone May 5, 2021
@DirectXMan12 DirectXMan12 deleted the bug/quote-envtest-tool-path branch May 7, 2021 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants