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.

Sign up for GitHub

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

Jump to bottom

fix wrapped response for auth.token.create #966

Merged
merged 7 commits into from Apr 1, 2023
Merged

fix wrapped response for auth.token.create #966

merged 7 commits into from Apr 1, 2023

Conversation

briantist
Copy link
Contributor

Fixes #965

  • Adds tests for wrapped responses
  • Ensures role-based tokens can be wrapped

(first run expected to fail: test added to catch the condition, fix not applied yet)

@briantist briantist added bug auth methods generally related to a Vault auth method token Related to the token auth method patch Used as part of release-drafter's version-resolver configuration labels Mar 26, 2023
@briantist briantist added this to the 1.1.1 milestone Mar 26, 2023
@briantist briantist self-assigned this Mar 26, 2023
@briantist briantist marked this pull request as ready for review March 26, 2023 18:58
@briantist briantist requested a review from a team as a code owner March 26, 2023 18:58
@codecov
Copy link

codecov bot commented Mar 26, 2023

Codecov Report

Merging #966 (34d7366) into develop (fba478b) will decrease coverage by 1.93%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop     #966      +/-   ##
===========================================
- Coverage    84.00%   82.08%   -1.93%     
===========================================
  Files           65       65              
  Lines         3020     3019       -1     
===========================================
- Hits          2537     2478      -59     
- Misses         483      541      +58
Impacted Files Coverage Δ
hvac/api/auth_methods/token.py 83.33% <100.00%> (-0.23%) ⬇️

... and 3 files with indirect coverage changes

@briantist briantist mentioned this pull request Mar 26, 2023
Open
adammike
adammike approved these changes Mar 27, 2023
View reviewed changes
Copy link
Member

@adammike adammike left a comment

Choose a reason for hiding this comment

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

LG2M, I also think the code looks much cleaner. Great job.

@briantist briantist merged commit 4868acc into hvac:develop Apr 1, 2023
35 checks passed
briantist added a commit to briantist/hvac that referenced this pull request May 10, 2023
@briantist
fix wrapped response for auth.token.create (hvac#966)
7d0494d 
* add policy and role test context managers

* ensure cleanup on role test

* add test for wrapped periodic token

* cleanup role test with context manager

* add test for wrapped role-based token

* slightly better assertions

* fix wrapped responses
briantist added a commit to briantist/hvac that referenced this pull request May 10, 2023
@briantist
fix wrapped response for auth.token.create (hvac#966)
82c8a23 
* add policy and role test context managers

* ensure cleanup on role test

* add test for wrapped periodic token

* cleanup role test with context manager

* add test for wrapped role-based token

* slightly better assertions

* fix wrapped responses
briantist added a commit to briantist/hvac that referenced this pull request May 10, 2023
@briantist
fix wrapped response for auth.token.create (hvac#966)
beafd33 
* add policy and role test context managers

* ensure cleanup on role test

* add test for wrapped periodic token

* cleanup role test with context manager

* add test for wrapped role-based token

* slightly better assertions

* fix wrapped responses
briantist added a commit that referenced this pull request Jun 17, 2023
@briantist
fix wrapped response for auth.token.create (#966)
458f724 
* add policy and role test context managers

* ensure cleanup on role test

* add test for wrapped periodic token

* cleanup role test with context manager

* add test for wrapped role-based token

* slightly better assertions

* fix wrapped responses
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blmhemu blmhemu blmhemu approved these changes

@adammike adammike adammike approved these changes

Assignees

@briantist briantist

Labels
auth methods generally related to a Vault auth method bug patch Used as part of release-drafter's version-resolver configuration token Related to the token auth method
Projects
None yet
Milestone
1.1.1
Development

Successfully merging this pull request may close these issues.

auth.token.create does not use wrap_ttl parameter if a role_name is specified
3 participants
@briantist @blmhemu @adammike