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

Add support for Azure Policy Assignment metadata #7725

Merged

Conversation

tyconsulting
Copy link
Contributor

@tyconsulting tyconsulting commented Jul 13, 2020

Resolves #7724 Add support for specifying meta data for Azure Policy Assignments. Same as Azure Policy Definitions, metadata is also supported by Policy Assignment in ARM. Currently, the azure_policy_assignment in azurerm provider (v2.18.0) does not support metadata as an input.

@ghost ghost added the size/S label Jul 13, 2020
@tyconsulting tyconsulting changed the title Sdd support for Azure Policy Assignment metadata Add support for Azure Policy Assignment metadata Jul 13, 2020
@tyconsulting
Copy link
Contributor Author

Feature Request: #7724

Copy link
Contributor

@ArcturusZhang ArcturusZhang left a comment

Choose a reason for hiding this comment

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

Hi @tyconsulting thanks for this PR!

This PR looks great to me, aside from some minor comments that I have left inline, please have a look, thanks!

Copy link
Member

@tombuildsstuff tombuildsstuff left a comment

Choose a reason for hiding this comment

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

hey @tyconsulting

Thanks for this PR :)

Taking a look through here this is looking good, if we can add this new field to the documentation (and fix the linting error) then this otherwise LGTM 👍

Thanks!

@ghost ghost added documentation size/M and removed size/S labels Jul 14, 2020
@tyconsulting
Copy link
Contributor Author

hey @tyconsulting

Thanks for this PR :)

Taking a look through here this is looking good, if we can add this new field to the documentation (and fix the linting error) then this otherwise LGTM 👍

Thanks!

Hi @tombuildsstuff thanks for getting back to me. i've updated the documentation. however, I'm struggling to find the error identified by tflint. when I ran "terrafmt fmt -f ./azurerm/internal/services/policy/tests/policy_assignment_resource_test.go -v", the output is:
./azurerm/internal/services/policy/tests/policy_assignment_resource_test.go: 606 lines & formatted 0/8 blocks!

It didn't update anything. Could you please check for me and let me know exactly where I need to change? thanks.

Copy link
Contributor

@ArcturusZhang ArcturusZhang left a comment

Choose a reason for hiding this comment

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

Hi @tyconsulting thanks for the revisions!

There are two minor format inline comments, please have a look.

And as for the terrafmt, I just run

terrafmt fmt -f .\azurerm\internal\services\policy\tests\policy_assignment_resource_test.go

and it works fine, you could have a try (without -v). Or you could try make terrafmt which would also works (but it may take a long time to run)

website/docs/r/policy_assignment.html.markdown Outdated Show resolved Hide resolved
website/docs/r/policy_assignment.html.markdown Outdated Show resolved Hide resolved
@ghost ghost added size/S and removed size/M labels Jul 15, 2020
Copy link
Contributor

@ArcturusZhang ArcturusZhang left a comment

Choose a reason for hiding this comment

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

Thanks @tyconsulting for the revisions, now LGTM 👍

Copy link
Member

@jackofallops jackofallops left a comment

Choose a reason for hiding this comment

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

Hi @tyconsulting
Thanks for this. I've taken a quick run though your changes and I've added a couple of comments that will need addressing before we can merge.

Thanks again!

Co-authored-by: Steve <11830746+jackofallops@users.noreply.github.com>
@ghost ghost added size/M and removed size/S labels Jul 16, 2020
@jackofallops
Copy link
Member

Tests passing:
image

@jackofallops jackofallops added this to the v2.19.0 milestone Jul 16, 2020
@jackofallops jackofallops merged commit 5aef312 into hashicorp:master Jul 16, 2020
jackofallops added a commit that referenced this pull request Jul 16, 2020
@ghost
Copy link

ghost commented Jul 16, 2020

This has been released in version 2.19.0 of the provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading. As an example:

provider "azurerm" {
    version = "~> 2.19.0"
}
# ... other configuration ...

@ghost
Copy link

ghost commented Aug 15, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 hashibot-feedback@hashicorp.com. Thanks!

@ghost ghost locked and limited conversation to collaborators Aug 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for Policy Assignment metadata
5 participants