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

Consul session token #5193

Merged

Conversation

wilfriedroset
Copy link
Contributor

SUMMARY

This change allow to use a token when creating a session.
It also fixes the fact that session_id was taken as an argument of consul_kv but not applied when creating a new key/pair.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

consul_kv
consul_session

ADDITIONAL INFORMATION

Before

PLAY [Example] *************************************************
TASK [Create a session with a token] ******************************
fatal: [localhost]: FAILED! => {"changed": false, "msg": "Unsupported parameters for (community.general.consul_session) module: token. Supported parameters include: delay, id, node, scheme, datacenter, checks, validate_certs, behavior, name, port, host, ttl, state."}
NO MORE HOSTS LEFT *************************************************************
PLAY RECAP *********************************************************************localhost : ok=0    changed=0    unreachable=0    failed=1    skipped=1    rescued=0    ignored=0

After

PLAY [Example] *************************************************
TASK [Create a session with a token] ******************************
changed: [localhost]
PLAY RECAP *********************************************************************localhost : ok=3    changed=1    unreachable=0    failed=0    skipped=1    rescued=0    ignored=0

@ansibullbot
Copy link
Collaborator

@ansibullbot ansibullbot added bug This issue/PR relates to a bug clustering module module plugins plugin (any type) labels Aug 26, 2022
@github-actions
Copy link

github-actions bot commented Aug 26, 2022

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

Copy link
Collaborator

@felixfontein felixfontein 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 your contribution! I've added some comments below.

changelogs/fragments/5193-consul-session-token.yaml Outdated Show resolved Hide resolved
plugins/modules/clustering/consul/consul_session.py Outdated Show resolved Hide resolved
changelogs/fragments/5193-consul-session-token.yaml Outdated Show resolved Hide resolved
@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Aug 27, 2022
@wilfriedroset
Copy link
Contributor Author

Thank you for your review, all suggestions have been taken into account.

Copy link
Collaborator

@felixfontein felixfontein left a comment

Choose a reason for hiding this comment

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

It's probably a good idea to split this up into two PRs. They concern two completely unrelated things.

plugins/modules/clustering/consul/consul_session.py Outdated Show resolved Hide resolved
plugins/modules/clustering/consul/consul_kv.py Outdated Show resolved Hide resolved
Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>
@felixfontein
Copy link
Collaborator

I'll keep this open for a bit in case someone else wants to comment, but if nobody comments I'll merge :)

@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Sep 3, 2022
@felixfontein felixfontein merged commit feabe20 into ansible-collections:main Sep 3, 2022
@patchback
Copy link

patchback bot commented Sep 3, 2022

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/feabe20c63b8845290baad674d55e439c4bf5445/pr-5193

Backported as #5222

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Sep 3, 2022
Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>

Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>
(cherry picked from commit feabe20)
@felixfontein
Copy link
Collaborator

@wilfriedroset thanks a lot for contributing this!

felixfontein pushed a commit that referenced this pull request Sep 3, 2022
Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>

Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>
(cherry picked from commit feabe20)

Co-authored-by: wilfriedroset <wilfriedroset@users.noreply.github.com>
bratwurzt pushed a commit to bratwurzt/community.general that referenced this pull request Nov 7, 2022
Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>

Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>
bratwurzt pushed a commit to bratwurzt/community.general that referenced this pull request Nov 7, 2022
Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>

Signed-off-by: Wilfried Roset <wilfriedroset@users.noreply.github.com>
This was referenced Nov 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug clustering module module plugins plugin (any type)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants