-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat(policies): support storage policies within terraform #558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will need to run make docs
locally, otherwise left a couple of inline comments, some of them should remove a number of subtype checks performed here.
var policy_type string | ||
switch { | ||
case strings.HasPrefix(policyId, policyStoragePolicyPrefix): | ||
if _, err := scp.AttachStoragePolicy(ctx, scopeId, 0, policyId, opts...); err != nil { | ||
return diag.FromErr(err) | ||
} | ||
policy_type = "storage" | ||
default: | ||
return diag.Errorf("unknown policy type provided.") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure we need the check here, but this should only ever support storage policy types so if you really want a check you can just do the if strings.HasPrefix
, though if you are doing this check you should check that the scopeId is global
or that of an org scope only as well
987270b
to
4e86cae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One new comment and responded to the one thread re subtype agnostic attachments. Regardless on the final approach there we should add scope
to the resource so resource_scope_policy_attachment
or resource_scope_policy_storage_attachment
58a9855
to
de1f0ce
Compare
de1f0ce
to
793fa38
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great!
793fa38
to
4e69d50
Compare
NOTE: requires hashicorp/boundary#4385 to be merged and tagged and go.mod updated before merging and deploying
testing was done manually due to policies being an enterprise feature