Added data source for WAF policy (fixes #7468) #7469

rikribbers · 2020-06-24T13:01:11Z

Should fix #7468

However being a newbee go programmer I might need some help getting it up and running. I have a working execution plan with an data source but I am ~~in doubt if~~ alsmost sure now that the resource schema for reading is correct... any comments highly appreciated.

Added data source for WAF policy Added documentation Docs. picked one Added link Updates udates Docs Nailed it

rikribbers · 2020-06-25T10:46:17Z

To my best of knowledge this pull request is done. However @mbfrahry you made some changes to the changelog (that I saw after squashing my "trial and error" commits getting to know the code/toolset). These seem unrelevant as they are related to a different resource (azurerm_role_definition)

Another question is what is needed to get this merged into master? The data source is something we actually want to use to seperate the code base for WAF policies and application gateways in Azure.

tombuildsstuff

hey @rikribbers

Thanks for this PR :)

Taking a look through this mostly LGTM - if we can remove the changelog entry (and the tests pass) then this should otherwise be good to merge 👍

Thanks!

CHANGELOG.md

rikribbers · 2020-06-25T14:17:06Z

@tombuildsstuff Thx for your respone, I have removed the changelog entry.

tombuildsstuff

LGTM - thanks for this @rikribbers

tombuildsstuff

hey @rikribbers

Apologies I just noticed there's no test for this new data source - would you mind adding one covering this? I've added an example inline - but this should otherwise be good to merge 👍

Thanks!

azurerm/internal/services/network/web_application_firewall_policy_data_source.go

rikribbers · 2020-06-26T14:38:17Z

@tombuildsstuff I have added an acceptance test. Could you verify if it is correct?

rikribbers · 2020-06-29T12:19:28Z

I see that there are a few pending check including the travis build, however if I follow the link the build is succesful... is there something broken or am I missing something here....

tombuildsstuff

hey @rikribbers

Thanks for adding a test here, apologies for the delayed re-review!

Taking a look this is mostly looking good now, if we can update the Test Configuration then the tests should pass and we should be able to get this in 👍

Thanks!

azurerm/internal/services/network/tests/web_application_firewall_policy_data_source_test.go

rikribbers · 2020-07-01T11:35:43Z

@tombuildsstuff updated the test. Should have seen these changes myself using the provider on a daily basis.... will look better next time ;-)

manicminer

Hey @rikribbers, thanks for the changes. I'm getting some errors running the test, have added some suggestions inline :)

azurerm/internal/services/network/tests/web_application_firewall_policy_data_source_test.go

azurerm/internal/services/network/web_application_firewall_policy_data_source.go

azurerm/internal/services/network/tests/web_application_firewall_policy_data_source_test.go

rikribbers · 2020-07-07T15:42:28Z

Thx for the reply, will look into it. I was wondering how I can run only this test locally. so I can verify stuff is working correctly.

manicminer · 2020-07-07T15:58:11Z

You'll need to supply an Azure subscription and tests do incur usage costs. This should do it:

make testacc TEST=./azurerm/internal/services/network/tests/ TESTARGS='-run TestAccDataSourceAzureRMWebApplicationFirewallPolicy_'

Note I did run the test myself with my suggested changes and it passed.

rikribbers · 2020-07-07T16:03:28Z

Again many thx for helping, I am just getting to know how the provider is actually working. I want to do thing right but also understand them...

…ll_policy_data_source_test.go Co-authored-by: Tom Bamford <[email protected]>

…icy_data_source.go Co-authored-by: Tom Bamford <[email protected]>

rikribbers · 2020-07-07T17:28:25Z

Hello @manicminer and @tombuildsstuff as far as I can tell this is now done. I have run the acceptance test successfully locally. Many thanks to you and the provider team for helping me figure out how this works. With this (and another change) merged I have learned a lot about go, the azurerm provider and once released we have the perfect solution for managing web application firewall policies in combination with application gateways. ❤️

manicminer · 2020-07-07T21:21:39Z

@rikribbers No problem at all, great to welcome you as a contributor :)

I'm just running the acceptance tests on our CI server, but this looks good and it's passing locally for me.

manicminer · 2020-07-07T21:37:17Z

Test results (new data source and existing resource):

ghost · 2020-07-10T01:21:38Z

This has been released in version 2.18.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.18.0"
}
# ... other configuration ...

ghost · 2020-08-07T14:51:26Z

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 👉 [email protected]. Thanks!

ghost added size/S size/M documentation size/XL and removed size/S size/M size/XL labels Jun 24, 2020

mbfrahry and others added 2 commits June 25, 2020 08:25

Update changelog for #7424

4ed5259

Added data source for WAF policy Added documentation Docs. picked one Added link Updates udates Docs Nailed it

removed whitespace

0a2e249

tombuildsstuff suggested changes Jun 25, 2020

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

tombuildsstuff added this to the v2.17.0 milestone Jun 25, 2020

tombuildsstuff added new-data-source service/firewall waiting-response labels Jun 25, 2020

remove changelog change

428a477

ghost removed the waiting-response label Jun 25, 2020

tombuildsstuff approved these changes Jun 25, 2020

View reviewed changes

tombuildsstuff suggested changes Jun 25, 2020

View reviewed changes

azurerm/internal/services/network/web_application_firewall_policy_data_source.go Show resolved Hide resolved

tombuildsstuff added the waiting-response label Jun 25, 2020

Updated

1fa619c

ghost added size/L and removed size/M labels Jun 25, 2020

rikribbers added 3 commits June 26, 2020 15:01

working unittest

6d898c6

Updates

e0fa665

formatting

fcd9ff0

ghost removed the waiting-response label Jun 26, 2020

rikribbers requested a review from tombuildsstuff June 29, 2020 12:01

tombuildsstuff suggested changes Jul 1, 2020

View reviewed changes

tombuildsstuff added the waiting-response label Jul 1, 2020

Updated test

1d10725

ghost removed the waiting-response label Jul 1, 2020

udpates

5902a53

katbyte modified the milestones: v2.17.0, v2.18.0 Jul 3, 2020

rikribbers requested a review from tombuildsstuff July 3, 2020 14:35

manicminer suggested changes Jul 7, 2020

View reviewed changes

rikribbers and others added 3 commits July 7, 2020 18:48

Update azurerm/internal/services/network/tests/web_application_firewa…

453d850

…ll_policy_data_source_test.go Co-authored-by: Tom Bamford <[email protected]>

Update azurerm/internal/services/network/web_application_firewall_pol…

8089420

…icy_data_source.go Co-authored-by: Tom Bamford <[email protected]>

Working ACC test

26cafd8

manicminer approved these changes Jul 7, 2020

View reviewed changes

manicminer merged commit ab38603 into hashicorp:master Jul 7, 2020

manicminer added a commit that referenced this pull request Jul 7, 2020

update changelog for #7469

38a5e20

ghost locked and limited conversation to collaborators Aug 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added data source for WAF policy (fixes #7468) #7469

Added data source for WAF policy (fixes #7468) #7469

rikribbers commented Jun 24, 2020 •

edited

Loading

rikribbers commented Jun 25, 2020

tombuildsstuff left a comment

rikribbers commented Jun 25, 2020

tombuildsstuff left a comment

tombuildsstuff left a comment

rikribbers commented Jun 26, 2020

rikribbers commented Jun 29, 2020

tombuildsstuff left a comment

rikribbers commented Jul 1, 2020

manicminer left a comment

rikribbers commented Jul 7, 2020

manicminer commented Jul 7, 2020

rikribbers commented Jul 7, 2020

rikribbers commented Jul 7, 2020

manicminer commented Jul 7, 2020

manicminer commented Jul 7, 2020 •

edited

Loading

ghost commented Jul 10, 2020

ghost commented Aug 7, 2020

Added data source for WAF policy (fixes #7468) #7469

Added data source for WAF policy (fixes #7468) #7469

Conversation

rikribbers commented Jun 24, 2020 • edited Loading

rikribbers commented Jun 25, 2020

tombuildsstuff left a comment

Choose a reason for hiding this comment

rikribbers commented Jun 25, 2020

tombuildsstuff left a comment

Choose a reason for hiding this comment

tombuildsstuff left a comment

Choose a reason for hiding this comment

rikribbers commented Jun 26, 2020

rikribbers commented Jun 29, 2020

tombuildsstuff left a comment

Choose a reason for hiding this comment

rikribbers commented Jul 1, 2020

manicminer left a comment

Choose a reason for hiding this comment

rikribbers commented Jul 7, 2020

manicminer commented Jul 7, 2020

rikribbers commented Jul 7, 2020

rikribbers commented Jul 7, 2020

manicminer commented Jul 7, 2020

manicminer commented Jul 7, 2020 • edited Loading

ghost commented Jul 10, 2020

ghost commented Aug 7, 2020

rikribbers commented Jun 24, 2020 •

edited

Loading

manicminer commented Jul 7, 2020 •

edited

Loading