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

cli: Fix broken KV import on Windows #10820

Merged
merged 2 commits into from
Aug 10, 2021
Merged

Conversation

blake
Copy link
Member

@blake blake commented Aug 10, 2021

Consul 1.10 (PR #9792) introduced the ability to specify a prefix when importing KV's. This however introduced a regression on Windows systems which breaks kv import. The key name is joined with specified-prefix using filepath.Join() which uses a forward slash (/) to delimit values on Unix-based systems, and a backslash () to delimit values on Windows – the latter of which is incompatible with Consul KV paths.

This commit replaces filepath.Join() with path.Join() which uses a forward slash as the delimiter, providing consistent key join behavior across supported operating systems.

Fixes #10583

Note: No additional tests were added in this commit because the existing tests would have caught this error if they had been run on Windows.

Consul 1.10 (PR #9792) introduced the ability to specify a prefix when
importing KV's. This however introduced a regression on Windows
systems which breaks `kv import`. The key name is joined with
specified`-prefix` using `filepath.Join()` which uses a forward slash
(/) to delimit values on Unix-based systems, and a backslash (\) to
delimit values on Windows – the latter of which is incompatible with
Consul KV paths.

This commit replaces filepath.Join() with path.Join() which uses a
forward slash as the delimiter, providing consistent key join behavior
across supported operating systems.

Fixes #10583
@blake blake added type/bug Feature does not function as expected theme/cli Flags and documentation for the CLI interface theme/windows Anything related to Windows backport/1.8 labels Aug 10, 2021
@blake blake requested a review from a team August 10, 2021 18:56
@blake blake self-assigned this Aug 10, 2021
@blake blake added theme/kv Issues related to the key value store theme/cli Flags and documentation for the CLI interface backport/1.10 and removed theme/cli Flags and documentation for the CLI interface backport/1.8 labels Aug 10, 2021
@vercel vercel bot temporarily deployed to Preview – consul August 10, 2021 19:44 Inactive
@vercel vercel bot temporarily deployed to Preview – consul-ui-staging August 10, 2021 19:44 Inactive
Copy link
Contributor

@dnephin dnephin left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@blake blake merged commit 1ee8655 into main Aug 10, 2021
@blake blake deleted the blake/fix-kv-import-windows-10583 branch August 10, 2021 21:42
@hc-github-team-consul-core
Copy link
Collaborator

🍒 If backport labels were added before merging, cherry-picking will start automatically.

To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/425057.

@hc-github-team-consul-core
Copy link
Collaborator

🍒✅ Cherry pick of commit 1ee8655 onto release/1.10.x succeeded!

hc-github-team-consul-core pushed a commit that referenced this pull request Aug 10, 2021
Consul 1.10 (PR #9792) introduced the ability to specify a prefix when
importing KV's. This however introduced a regression on Windows
systems which breaks `kv import`. The key name is joined with
specified`-prefix` using `filepath.Join()` which uses a forward slash
(/) to delimit values on Unix-based systems, and a backslash (\) to
delimit values on Windows – the latter of which is incompatible with
Consul KV paths.

This commit replaces filepath.Join() with path.Join() which uses a
forward slash as the delimiter, providing consistent key join behavior
across supported operating systems.

Fixes #10583
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/cli Flags and documentation for the CLI interface theme/kv Issues related to the key value store theme/windows Anything related to Windows type/bug Feature does not function as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

KV Import fails on Windows using Consul version 1.10.0
3 participants