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

Make policies importable. #15

Merged
merged 5 commits into from
Aug 30, 2017
Merged

Make policies importable. #15

merged 5 commits into from
Aug 30, 2017

Conversation

paddycarver
Copy link
Contributor

This makes policies importable. Minor changes were needed for this:

  • Read now sets name, in addition to policy.
  • We added the Importer functionality and a test for it.
  • Our test config needed to be updated to allow the policy name to be
    injected, to help ensure uniqueness across runs.

This makes policies importable. Minor changes were needed for this:

* Read now sets name, in addition to policy.
* We added the Importer functionality and a test for it.
* Our test config needed to be updated to allow the policy name to be
  injected, to help ensure uniqueness across runs.
@paddycarver
Copy link
Contributor Author

Test failure is an acctest vendoring failure. Should be resolved by #12.

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

Question about when we set the name

)

func TestAccPolicy_importBasic(t *testing.T) {
name := "test-" + acctest.RandString(10)
Copy link
Contributor

Choose a reason for hiding this comment

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

There's also RandomWithPrefix, if integers are valid in the name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL! Thanks :)

@@ -77,6 +80,7 @@ func policyRead(d *schema.ResourceData, meta interface{}) error {
}

d.Set("policy", policy)
d.Set("name", name)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we set this in the CREATE method, policyWrite? Right now, policyWrite does not call policyRead, so the name attribute won't be set until a refresh operation happens.

Normally we call create and it returns read. If that's not necessary here, I recommend we set name in policyWrite then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I missed that wasn't happening here. Updated to now call policyRead from policyWrite.

Copy link
Contributor

@catsby catsby left a comment

Choose a reason for hiding this comment

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

👍

@paddycarver paddycarver merged commit aedd25e into master Aug 30, 2017
@tyrannosaurus-becks tyrannosaurus-becks deleted the paddy_import_policy branch February 16, 2019 00:20
dandandy pushed a commit to dandandy/terraform-provider-vault that referenced this pull request Jun 17, 2021
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.

2 participants