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

Configurable list of headers to avoid writing to tfstate #12

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hynd
Copy link

@hynd hynd commented Jun 13, 2018

I'd like to avoid writing some headers (eg; "Authorization") to Terraform state...
Not sure what you think about the config name?

The implementation is a little... unusual - it doesn't seem like StateFunc applies early enough to data sources (vs resources), so i had to resort to Set() to overwrite the state in the Read function
(ditto for ImportState et al for testing state set by a data source).

@apparentlymart
Copy link
Member

Hi @hynd! Thanks for working on this.

We're currently working on some changes to how exactly Terraform Core and providers will interact when processing changes, with development in a feature branch in the main repository. Since this change is doing something slightly unusual (modifying how a value from configuration is stored in state) I'd like to hold on this change for the moment until we've finished that work in progress, and then we can see how this interacts with it.

Usually providers only use d.Set with values that are marked as Computed in the schema, to provide the resulting value. In principle what you've done here is harmless because the data source will be re-read on each refresh anyway, but since this has (as far as I know) never been tried before, and we're making some changes that presume existing usage patterns, I want to make sure this isn't going to become broken in some way by our forthcoming changes. If it is, then we can hopefully find a different way to meet the use-case, such as providing per-host credentials to the http provider out of band of the data source itself.

For other providers, we usually recommend using non-Terraform-specific means to pass credentials, so they can be set for your session and apply to other tools that might talk to the same API. This usual practice isn't a perfect fit for the http provider because the credentials are necessarily per-host and unlikely to be useful for session-wide configuration, but we'd have a few other options here, and we could potentially support multiple of them:

  • Make the provider support .netrc files, and allow you to specify one or more files in the provider configuration itself, to be used across all http resources where the hostname matches.

  • Support a new host block inside the provider block to allow credentials to be provided inline, allowing the same sort of interpolation patterns you can do directly in the resource:

    provider "http" {
      host "example.com" {
        username = "foo"
        password = "bar"
        # Could also allow setting the authorization header directly as an alternative here,
        # for other schemes like "Bearer".
      }
    }

Once the situation looks clearer with the provider protocol changes in progress I'll take another look here and test how your technique is behaving under the new assumptions.

Thanks again for working on this!

@hynd
Copy link
Author

hynd commented Jun 13, 2018

I had similar thoughts about alternate ways of providing credentials (inc envvars) and multiple hosts, but couldn't think of anything nice :/

Thanks for the info about the refactor - i'll hang ten!

@sambhaji-dhawan
Copy link

@apparentlymart - Any updates on the approach here?

@hashicorp-cla
Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

Base automatically changed from master to main February 1, 2021 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants