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

29 support all provider attributes in .tf files #59

Merged
merged 3 commits into from
Apr 27, 2020

Conversation

sgnn7
Copy link
Contributor

@sgnn7 sgnn7 commented Apr 24, 2020

This PR:

  • Adds support for setting these directly in .tf files:
    • appliance_url
    • account
    • ssl_cert
    • ssl_cert_file
  • Adds SSL to Conjur OSS tests
  • Adds tests for use with only .tf variables
  • Updates CHANGELOG/README

Connected to #29

sgnn7 added 2 commits April 24, 2020 11:34
We now support setting these two variables in the proper place for a
terraform configuration rather than relying on env vars to do it for us.
This allows allsettings to be set through the `.tf` file now. We also
changed the oss tests to include SSL as well.
@sgnn7 sgnn7 marked this pull request as ready for review April 24, 2020 18:45
Copy link
Contributor

@izgeri izgeri left a comment

Choose a reason for hiding this comment

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

a few questions / comments

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@izgeri izgeri left a comment

Choose a reason for hiding this comment

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

just one last comment to add some clarity to the README

it's worth also noting that as of right now we don't have tests for when there is a mix of provider / env configuration settings, or for when the ssl_cert_path is used - is there a way to add unit tests for that? if it's not straightforward, should we file a follow-up github issue?

@sgnn7 sgnn7 force-pushed the 29-support-all-provider-attributes branch from 13339dd to 93bcc1f Compare April 27, 2020 15:12
Examples used completely different info between them so correlation was
difficult. This change ensures that all of them use the same info.
@sgnn7 sgnn7 force-pushed the 29-support-all-provider-attributes branch from 93bcc1f to f8a691d Compare April 27, 2020 15:21
@sgnn7
Copy link
Contributor Author

sgnn7 commented Apr 27, 2020

@izgeri Updated again. Opened an issue for unit tests (#60) since we don't have any 😢.

@izgeri izgeri mentioned this pull request Apr 27, 2020
1 task
@izgeri izgeri merged commit 4a15d7c into master Apr 27, 2020
@izgeri izgeri deleted the 29-support-all-provider-attributes branch April 27, 2020 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants