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

feat: support custom idp login when recording test fixtures #247

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

kuntzed
Copy link
Contributor

@kuntzed kuntzed commented Jun 28, 2023

Purpose

  • Add support for setting the identity provider from environment variable BTP_IDP during provider configuration. This is implemented analogously to BTP_USERNAME and BTP_PASSWORD including the required redaction of recorded fixtures. To prevent impact on existing fixtures, identity provider values are replaced by the empty string during redaction.

Does this introduce a breaking change?

[ ] Yes
[x] No

Pull Request Type

What kind of change does this Pull Request introduce?

[ ] Bugfix
[x] Feature
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

How to Test

  • Test the code

1. go test ./...
2. Try local recording of text fixtures and use a custom idp user by setting all three of the env vars BTP_USERNAME, BTP_PASSWORD, BTP_IDP, setting the latter to the ias tenant 'terraformint'

What to Check

Verify that the following are valid

  1. The tests should still be successful
  2. The resulting fixtures should not contain any information about the selected idp (ias tenant)

Other Information

Also fixes a warning when the username has an unknown value during provider configuration.

Resolves #243

Add support for setting the identity provider from environment
variable BTP_IDP during provider configuration. This is implemented
analogously to BTP_USERNAME and BTP_PASSWORD including the required
redaction of recorded fixtures. To prevent impact on existing
fixtures, identity provider values are replaced by the empty string
during redaction.

Also fixes a warning when the username has an unknown value during
provider configuration.

Resolves #243
@kuntzed kuntzed self-assigned this Jun 28, 2023
@kuntzed
Copy link
Contributor Author

kuntzed commented Jun 28, 2023

We should discuss if/when it would make sense to adapt the request redaction to replace any idp with some test value instead of the empty string, although this would mean that all fixtures need to be re-recorded. While at it, we could also replace the issuer returned during login to a test value to get rid of the hardcoded values containing 'accounts.sap.com' and 'accounts400.ondemand.com'.

@lechnerc77 lechnerc77 added the ignore-for-release All things not to be mentioned in release notes label Jun 28, 2023
@lechnerc77 lechnerc77 added this to the 0.2.0-beta1 - July release milestone Jun 28, 2023
@lechnerc77 lechnerc77 requested a review from v0lkc June 28, 2023 15:20
@lechnerc77 lechnerc77 enabled auto-merge (squash) June 28, 2023 15:20
internal/provider/provider.go Outdated Show resolved Hide resolved
internal/provider/provider_test.go Outdated Show resolved Hide resolved
internal/provider/provider_test.go Show resolved Hide resolved
@lechnerc77 lechnerc77 requested a review from v0lkc July 5, 2023 09:53
@lechnerc77 lechnerc77 requested review from v0lkc and removed request for v0lkc July 6, 2023 09:15
@lechnerc77 lechnerc77 disabled auto-merge July 7, 2023 07:58
@lechnerc77 lechnerc77 merged commit 30773b4 into main Jul 7, 2023
12 of 13 checks passed
@lechnerc77 lechnerc77 deleted the issue243 branch July 7, 2023 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ignore-for-release All things not to be mentioned in release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Support recording of test fixtures using a BTP user coming from a custom identity provider
3 participants