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

Add to k8s_raw docs re: vault-encrypted files #47328

Merged
merged 5 commits into from
Oct 28, 2018
Merged

Conversation

r4j4h
Copy link
Contributor

@r4j4h r4j4h commented Oct 19, 2018

I didn't read the examples far enough and maybe would not have tried all the examples so I ended up creating #47259 and learned that definition key with lookup works well and we agreed the docs should say something. :)

+label: docsite_pr

SUMMARY

Fixes #47259

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

k8s_raw

ANSIBLE VERSION
2.7
ADDITIONAL INFORMATION

I didn't read the examples far enough and maybe would not have tried all the examples so I ended up creating ansible#47259 and learned that definition key with lookup works well and we agreed the docs should say something. :)

+label: docsite_pr
@ansibot
Copy link
Contributor

ansibot commented Oct 19, 2018

Hi @r4j4h, thank you for submitting this pull-request!

click here for bot help

@ansibot
Copy link
Contributor

ansibot commented Oct 19, 2018

@ansibot ansibot added affects_2.8 This issue/PR affects Ansible v2.8 community_review In order to be merged, this PR must follow the community review workflow. docs This issue/PR relates to or includes documentation. k8s module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. small_patch support:community This issue/PR relates to code supported by the Ansible community. labels Oct 19, 2018
@willthames
Copy link
Contributor

Thanks for doing this - this is good, I think it could be improved further by mentioning in the description of src that it doesn't cope with vaulted files

@ansibot ansibot removed the needs_triage Needs a first human triage before being processed. label Oct 19, 2018
@r4j4h
Copy link
Contributor Author

r4j4h commented Oct 19, 2018

I would love to, but I can't seem to find where the description of src is actually generated.. I only see merge_type in the doc file and I can't seem to find where the rest are being defined. Can you point me in the right direction?

@r4j4h
Copy link
Contributor Author

r4j4h commented Oct 19, 2018

Of course after I post I think of a more clever way to find it. How does that look? Felt wrong to mash it with the controller file description since only the lookup is needed so I added that as another bullet point instead.

@ansibot ansibot added core_review In order to be merged, this PR must follow the core review workflow. support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed community_review In order to be merged, this PR must follow the community review workflow. small_patch labels Oct 19, 2018
@dagwieers dagwieers added the docsite_pr This PR is created from documentation using the "Edit on GitHub" link. label Oct 19, 2018
Copy link
Contributor

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I just have a few suggestions for phrasing. I'm not very familiar with vault, so my apologies if my suggestions are technically incorrect.

lib/ansible/modules/clustering/k8s/k8s.py Outdated Show resolved Hide resolved
lib/ansible/modules/clustering/k8s/k8s.py Outdated Show resolved Hide resolved
Copy link
Contributor

@fabianvf fabianvf left a comment

Choose a reason for hiding this comment

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

I just realised that I probably shouldn't mark something with Request changes while I'm traveling and might not be able to adjust my review in a reasonable amount of time. Switching to Approve just so that I don't gum up the wheels of progress.

@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed core_review In order to be merged, this PR must follow the core review workflow. labels Oct 25, 2018
@r4j4h
Copy link
Contributor Author

r4j4h commented Oct 28, 2018

@willthames @fabianvf Great feedback, I will make the suggested changes soon :)

@willthames willthames merged commit 303fa6b into ansible:devel Oct 28, 2018
@willthames
Copy link
Contributor

Thanks @r4j4h for making the docs better!

@r4j4h
Copy link
Contributor Author

r4j4h commented Nov 9, 2018

It was truly my pleasure @willthames :) I'm glad I could help and hope to more in time. 💖

Tomorrow9 pushed a commit to Tomorrow9/ansible that referenced this pull request Dec 4, 2018
* Add to k8s_raw docs re: vault-encrypted files

I didn't read the examples far enough and maybe would not have tried all the examples so I ended up creating ansible#47259 and learned that definition key with lookup works well and we agreed the docs should say something. :)

+label: docsite_pr

* Add note about ansible vault-encrypted files to src: param

* Remove trailing whitespace

* Make changes from feedback

* Make feedback changes
@ansible ansible locked and limited conversation to collaborators Jul 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.8 This issue/PR affects Ansible v2.8 docs This issue/PR relates to or includes documentation. docsite_pr This PR is created from documentation using the "Edit on GitHub" link. k8s module This issue/PR relates to a module. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:community This issue/PR relates to code supported by the Ansible community. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

k8s_raw cannot handle Ansible Vault encrypted files
5 participants