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

Only lookup secrets in the namespace where the install is targeting. #1156

Merged
merged 2 commits into from Apr 8, 2022

Conversation

t-eckert
Copy link
Contributor

@t-eckert t-eckert commented Apr 8, 2022

Changes proposed in this PR:

  • The install command will only look for secrets in the namespace the install is targeting.
    This fixes the issue raised by @lkysow.

    another one, it's complaining I have a secret in my default ns but I'm installing consul into the consul ns:

    consul-k8s install
    ==> Checking if Consul can be installed
    ✓ No existing Consul installations found.
    ✓ No existing Consul persistent volume claims found
    ! Found Consul secrets, possibly from a previous installation.
    Delete existing Consul secrets from Kubernetes:
    
    kubectl delete secret consul-federation --namespace default

How I've tested this PR:

Extended existing unit tests to cover this case.

How I expect reviewers to test this PR:

👀 and running unit tests.

Checklist:

  • Tests added
  • CHANGELOG entry added

    HashiCorp engineers only, community PRs should not add a changelog entry.
    Entries should use present tense (e.g. Add support for...)

@t-eckert t-eckert force-pushed the cli/look-for-secrets-in-current-ns branch from 17c1120 to 84dcd13 Compare April 8, 2022 18:51
@t-eckert t-eckert requested review from lkysow and jmurret April 8, 2022 18:53
Copy link
Contributor

@jmurret jmurret left a comment

Choose a reason for hiding this comment

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

Nice!

@t-eckert t-eckert merged commit 0e691ba into main Apr 8, 2022
@t-eckert t-eckert deleted the cli/look-for-secrets-in-current-ns branch April 8, 2022 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants