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

Provider config #943

Merged
merged 5 commits into from
Jan 21, 2021
Merged

Provider config #943

merged 5 commits into from
Jan 21, 2021

Conversation

Gaardsholt
Copy link
Contributor

This fixes #828

@tvoran tvoran self-requested a review January 20, 2021 22:53
Copy link
Contributor

@jasonodonnell jasonodonnell left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@jasonodonnell jasonodonnell merged commit ad2757c into hashicorp:master Jan 21, 2021
@tx-kstav
Copy link

@Gaardsholt How are you using this? According to documentation (https://www.vaultproject.io/docs/auth/jwt_oidc_providers) there are types other than string in the provider_config (see: "fetch_groups" and "fetch_user_info" get boolean values, "groups_recurse_max_depth" gets int value).

However, using this, everything gets turned into string which is then not accepted.

@Gaardsholt
Copy link
Contributor Author

@tx-kstav Ohh.. you are right, I only tested the Azure specific one, which is just a string.
I will have a look at it tomorrow, it should be fairly simple to fix.

@Kostavro
Copy link

@tx-kstav Ohh.. you are right, I only tested the Azure specific one, which is just a string.
I will have a look at it tomorrow, it should be fairly simple to fix.

I would appreciate your input on 953, as it's my first attempt at editing a tf provider.

@Gaardsholt
Copy link
Contributor Author

@tx-kstav Ohh.. you are right, I only tested the Azure specific one, which is just a string.
I will have a look at it tomorrow, it should be fairly simple to fix.

I would appreciate your input on 953, as it's my first attempt at editing a tf provider.

Personally I would avoid hardcoding the provider_config keys, as it would give us more work to do whenever the config changes.
I think we should be able to change the provider_config to this:

"provider_config": {
  Type:        schema.TypeMap,
  Optional:    true,
  Description: "Provider specific handling configuration",
},

But I will test it as soon as I get off work :)

@Kostavro
Copy link

Kostavro commented Jan 26, 2021

Thank you! I agree with your thought, if that works. I found it difficult to use the updated, local provider with my test, but will try to test too, after work.
(Plus I noticed a typo in my pr, will fix that too)

@Gaardsholt
Copy link
Contributor Author

@tx-kstav can you test if it works if you just use strings instead of the correct type, something like this:

resource "vault_jwt_auth_backend" "gsuite" {
  provider           = vault
  path               = "oidc"
  type               = "oidc"

  provider_config = {
    provider                 = "gsuite"
    gsuite_service_account   = "/path/to/service-account.json"
    gsuite_admin_impersonate = "[email protected]"
    fetch_groups             = "true"
    fetch_user_info          = "true"
    groups_recurse_max_depth = "5"
    user_custom_schemas      = "Education,Preferences"
  }
}

If that works, I will make a new pull request, so you can use the correct types.

@Kostavro
Copy link

Kostavro commented Jan 26, 2021

@Gaardsholt unfortunately that is how I found out the issue. String values are not accepted, the correct type is requested for each of the three "problematic" properties.
Moreover, trying my fix throws:

`

2021-01-26T11:49:39.481+0200 [DEBUG] plugin.terraform-provider-vault_v2.18.0_x4: 2021/01/26 11:49:39 [DEBUG] Updating auth oidc2 in Vault
2021/01/26 11:49:39 [DEBUG] vault_jwt_auth_backend.this: apply errored, but we're indicating that via the Error pointer rather than returning it: error updating configuration to Vault for path oidc2: json: unsupported type: schema.SchemaSetFunc
2021/01/26 11:49:39 [TRACE] EvalMaybeTainted: vault_jwt_auth_backend.this encountered an error during creation, so it is now marked as tainted
2021/01/26 11:49:39 [TRACE] EvalWriteState: recording 0 dependencies for vault_jwt_auth_backend.this
2021/01/26 11:49:39 [TRACE] EvalWriteState: writing current state object for vault_jwt_auth_backend.this
2021/01/26 11:49:39 [TRACE] EvalApplyProvisioners: vault_jwt_auth_backend.this is tainted, so skipping provisioning
2021/01/26 11:49:39 [TRACE] EvalMaybeTainted: vault_jwt_auth_backend.this was already tainted, so nothing to do
2021/01/26 11:49:39 [TRACE] EvalWriteState: recording 0 dependencies for vault_jwt_auth_backend.this
2021/01/26 11:49:39 [TRACE] EvalWriteState: writing current state object for vault_jwt_auth_backend.this
2021/01/26 11:49:39 [TRACE] vertex "vault_jwt_auth_backend.this": visit complete
2021/01/26 11:49:39 [TRACE] dag/walk: upstream of "provider[\"registry.terraform.io/hashicorp/vault\"] (close)" errored, so skipping
2021/01/26 11:49:39 [TRACE] dag/walk: upstream of "meta.count-boundary (EachMode fixup)" errored, so skipping
2021/01/26 11:49:39 [TRACE] dag/walk: upstream of "root" errored, so skipping
2021/01/26 11:49:39 [TRACE] statemgr.Filesystem: creating backup snapshot at terraform.tfstate.backup
2021/01/26 11:49:39 [TRACE] statemgr.Filesystem: state has changed since last snapshot, so incrementing serial to 5
2021/01/26 11:49:39 [TRACE] statemgr.Filesystem: writing snapshot at terraform.tfstate

2021/01/26 11:49:39 [TRACE] statemgr.Filesystem: removing lock metadata file .terraform.tfstate.lock.info
2021/01/26 11:49:39 [TRACE] statemgr.Filesystem: unlocking terraform.tfstate using fcntl flock
Error: error updating configuration to Vault for path oidc2: json: unsupported type: schema.SchemaSetFunc

on main.tf line 13, in resource "vault_jwt_auth_backend" "this":
13: resource "vault_jwt_auth_backend" "this" {

`

when trying to create a resource as you just wrote. This leads me to believe there is some bug in the provider code, but I haven't had the time to look into it yet.

@Kostavro
Copy link

Exact output of trying to use strings (or even the correct type): # vault_jwt_auth_backend.this will be created

  + resource "vault_jwt_auth_backend" "this" {
      + accessor           = (known after apply)
      + default_role       = "readonly"
      + id                 = (known after apply)
      + oidc_client_id     = "xxxxxxx"
      + oidc_client_secret = (sensitive value)
      + oidc_discovery_url = "xxxxxxx"
      + path               = "oidc"
      + provider_config    = {
          + "fetch_groups"             = "true"
          + "fetch_user_info"          = "true"
          + "groups_recurse_max_depth" = "5"
          + "gsuite_admin_impersonate" = "xxxxxx"
          + "gsuite_service_account"   = "/etc/config/vault-sa-key.json"
          + "provider"                 = "gsuite"
        }
      + tune               = (known after apply)
      + type               = "oidc"
    }

Plan: 1 to add, 0 to change, 0 to destroy.

Do you want to perform these actions?
  Terraform will perform the actions described above.
  Only 'yes' will be accepted to approve.

  Enter a value: yes

vault_jwt_auth_backend.this: Creating...

Error: error updating configuration to Vault for path oidc: Error making API request.

URL: PUT https://xxxxxxxxxxxxxx/v1/auth/oidc/config

Code: 400. Errors:

* invalid provider_config: error initializing "gsuite" provider_config: 3 error(s) decoding:

* 'fetch_groups' expected type 'bool', got unconvertible type 'string'
* 'fetch_user_info' expected type 'bool', got unconvertible type 'string'
* 'groups_recurse_max_depth' expected type 'int', got unconvertible type 'string'

  on main.tf line 1, in resource "vault_jwt_auth_backend" "this":
   1: resource "vault_jwt_auth_backend" "this" {

@Gaardsholt
Copy link
Contributor Author

I have been looking a bit at this, and to be honest I don't know what the best way of solving this would be.
Maybe someone from the Terraform/Vault team can pitch in?

@Kostavro
Copy link

It's ok @Gaardsholt , thanks for taking a look. I opened a new issue, hopefully it will be noticed :)

dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
* Rush B!

* .

* Provider config (hashicorp#1)

* Rush B!

* .

* added provider_config to the matchingJwtMountConfigOptions list
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.

Feature Request: resource_jwt_auth_backend - add provider_config
4 participants