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

[Phase 2] Include Auth Login blocks in multiplexed Provider configuration #2097

Merged

Conversation

vinay-gopalan
Copy link
Contributor

@vinay-gopalan vinay-gopalan commented Nov 22, 2023

Branches off of #2090 to complete updates to all auth login blocks so that they:

  • Do not make use of environment default funcs to set values
  • Do not have fields marked as Required (validation is handled internally)
  • Do not set defaults in the schema

Ensures that the build is stable and the provider configurations match up

Completes the PoC for a TF Framework resource with vault_password_policy/vault_password_policy_fw

@vinay-gopalan vinay-gopalan changed the base branch from main to VAULT-21288/auth-login-fwprovider November 22, 2023 19:26
defaultValue := os.Getenv(f.envVar)
if defaultValue == "" {
defaultValue = f.defaultVal
for _, envVar := range f.envVars {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This allows us to pass multiple options for the default environment variable and take the first value we find. Similar to what MultiEnvDefaultFunc is achieving here

for _, vf := range validators {
if err := vf(d); err != nil {
if err := vf(d, params); err != nil {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating the signature for validateFunc and passing in the params here allows us to maintain existing behavior for l.params=nil returned when validation fails. This way we don't have to update any existing tests, since we only update l.params once all validators have passed

@benashz benashz self-requested a review November 22, 2023 21:20
// Resource: UpdateSchemaResource(passwordPolicyResource()),
// PathInventory: []string{"/sys/policy/password/{name}"},
// },
"vault_password_policy": {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Uncommented for now to let CI be green 🟢 . This may be removed from the SDKv2 provider once we add tests for the new framework password policy resource and confirm it works as expected

@@ -21,7 +23,7 @@ type PasswordPolicyResource struct {
}

func (r *PasswordPolicyResource) Metadata(_ context.Context, req resource.MetadataRequest, resp *resource.MetadataResponse) {
resp.TypeName = req.ProviderTypeName + "_password_policy"
resp.TypeName = req.ProviderTypeName + "_password_policy_fw"
Copy link
Contributor Author

@vinay-gopalan vinay-gopalan Nov 22, 2023

Choose a reason for hiding this comment

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

Temporarily names suffixed with _fw to avoid duplicate resource names and have green CI 🟢 This can be renamed to simply vault_password_policy once we add the TF Framework tests / are ready to deprecate the SDKv2 resource with the same name

Description: "The identity's client ID.",
Validators: []validator.String{
stringvalidator.ConflictsWith(
path.MatchRelative().AtName(consts.FieldJWT),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Need to ensure this works the same as it does for SDKv2 equivalents here

@vinay-gopalan vinay-gopalan requested review from a team and removed request for benashz November 23, 2023 00:55
@fairclothjm fairclothjm force-pushed the VAULT-21288/all-auth-logins branch from 676ef32 to 859a6d5 Compare November 29, 2023 20:42
@fairclothjm
Copy link
Contributor

Thanks @vinay-gopalan ! I addressed the todos that you noted, squashed all the commits in this PR, force pushed and will merge to the base branch

run go mod tidy

register all auth logins to test build

add all auth login blocks and match schema implementations

set default for azure scope

address todos

- remove commented code; neither are read from env vars
- add validators
@fairclothjm fairclothjm force-pushed the VAULT-21288/all-auth-logins branch from 859a6d5 to 8a99eda Compare November 29, 2023 20:48
@fairclothjm fairclothjm merged commit 289a101 into VAULT-21288/auth-login-fwprovider Nov 29, 2023
10 checks passed
fairclothjm added a commit that referenced this pull request Dec 12, 2023
* setup framework provider schema

* handle defaults in configure

* WIP: handle auth login methods

* [Phase 2] Include Auth Login blocks in multiplexed Provider configuration (#2097)

make updates to all auth logins and fix build

run go mod tidy

register all auth logins to test build

add all auth login blocks and match schema implementations

set default for azure scope

address todos

- remove commented code; neither are read from env vars
- add validators

* remove set_namespace_from_token field

* add custom path validator

* configure namespace for vault client

* review comments

* add muxed provider acc tests

* use consts, set required fields and update tests

* add tests for multi envs and fix bug

* fix bug with overriding config val

* add custom URI validator for OIDC auth

* fix a few default mount types

* add comments and migration state example acc test

* remove dupe default entries; add validators

* remove client_auth docs

* add kerbos token and file path validators

* don't use double pointer and remove redundant return

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
fairclothjm added a commit that referenced this pull request Dec 12, 2023
…mplementation (#2067)

* implement the framework provider

* setup provider server factory

* setup framework provider schema

* handle defaults in configure

* [Phase 2] Complete the provider configuration setup (#2090)

* setup framework provider schema

* handle defaults in configure

* WIP: handle auth login methods

* [Phase 2] Include Auth Login blocks in multiplexed Provider configuration (#2097)

make updates to all auth logins and fix build

run go mod tidy

register all auth logins to test build

add all auth login blocks and match schema implementations

set default for azure scope

address todos

- remove commented code; neither are read from env vars
- add validators

* remove set_namespace_from_token field

* add custom path validator

* configure namespace for vault client

* review comments

* add muxed provider acc tests

* use consts, set required fields and update tests

* add tests for multi envs and fix bug

* fix bug with overriding config val

* add custom URI validator for OIDC auth

* fix a few default mount types

* add comments and migration state example acc test

* remove dupe default entries; add validators

* remove client_auth docs

* add kerbos token and file path validators

* don't use double pointer and remove redundant return

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>

---------

Co-authored-by: vinay-gopalan <86625824+vinay-gopalan@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants