`azurerm_application_gateway` - support for redirect rules #2908

RustyF · 2019-02-18T00:13:41Z

WIP. I still need to write the tests and update the docs but I'm creating this PR now so you guys can take an early look.

I'm a go newbie and now using GoLand after grief with VSCode. My formatting is screwed also :)

…I docs

katbyte

Hi @RustyF,

Had a quick glance and left a couple minor comments inline, but overall this is looking pretty good 🙂 let us know when it's ready for a proper review

azurerm/resource_arm_application_gateway.go

RustyF · 2019-02-21T09:11:35Z

Thanks @katbyte for that initial review and categorisation. Of course, I will make the suggested changes.

One question on semantic validation, there are more mutually exclusive settings now. Do you typically leave these to be checked by Azure or would you like me to try and add some validation in the code? E.g. you can't have a target listener and a target URL.

katbyte · 2019-02-22T00:30:54Z

@RustyF,

Usually we try to do as mch validation at the schema level so errors are caught during plan, or at the very least before trying the API as frequently the errors returned by azure are mysterious and unhelpful. But it's not always viable or required.

If you can add a check to flattenApplicationGatewayRedirectConfigurations() so an understandable error is floated up to the user that would suffice 🙂

azurerm/resource_arm_application_gateway.go

Remove conflicts (not usable for nested resources) Add validation in expansion funcs

…e (with target listener and target URL mix) (not run yet - local environment issues)

RustyF · 2019-02-28T00:07:59Z

Quick update, I've made all the suggested changes, added some validation. I've also added a couple of tests which should give it reasonable coverage. I haven't actually run the tests yet 😮 as I'm having unrelated make test problems but will try again tomorrow. Other than that, subject to obvious issues or things I've overlooked/not understood, it should be code-complete/stable.

tombuildsstuff

hey @RustyF

Thanks for pushing those changes - I've taken a look through and this is looking really good, if we can fix up the minor issues (which I've commented in-line) then we should be able to run the tests and get this merged 👍

Thanks!

azurerm/resource_arm_application_gateway.go

website/docs/r/application_gateway.html.markdown

…rgetURL is fully specified. Not sure yet what to do with mutually-exclusive SubResource * references.

RustyF · 2019-03-05T21:47:40Z

Update, the first acceptance test works, i.e. basic rule with re-direction to HTTP listener. It just takes a while to run and obviously timed out yesterday. I'm still diagnosing the path map redirection test.

…f specified Ensure that default HTTP settings and back-end pool settings are now optional on path URL map (to facilitate redirections) Tweak tests to reflect better understanding of path URL map

RustyF · 2019-03-06T00:40:49Z

Ran out of time today but think I've resolved the problems with path URL map-based redirects. Will run the acceptance tests tomorrow and confirm.

RustyF · 2019-03-06T23:00:06Z

Acceptance tests are passing so we are pretty much there now, I think 😉. I'm going to review the docs again as I haven't looked at them since the final tweaks.

RustyF · 2019-03-06T23:02:36Z

Please take a final look at the code. As a go newbie, I won't be doing idiomatic things yet.

…viously required properties are now optional.

RustyF · 2019-03-06T23:31:36Z

Doc updates done.

RustyF · 2019-03-09T15:51:15Z

Let me know if you need me to do anything?

katbyte

Thanks for the updates @RustyF,

Aside from a couple comments i've left inline this is looking good. My primary comment is i don't think you need the NilOrEmpty function, I think that code should just be inlined.

azurerm/helpers/validate/strings.go

azurerm/resource_arm_application_gateway.go

…abstraction

…rules

RustyF · 2019-03-17T16:36:31Z

I ran the acceptance tests again successfully.

katbyte

~~left comment on wrong pr~~

wrong PR

katbyte

Thanks for the updates @RustyF! this LGTM now 👍

RustyF · 2019-03-22T10:42:25Z

@katbyte , @tombuildsstuff thanks for your support and patience on this 👍

ghost · 2019-04-03T17:34:25Z

This has been released in version 1.24.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 = "~> 1.24.0"
}
# ... other configuration ...

OffColour · 2019-04-08T08:05:03Z

@RustyF Massive thanks for your work on this. Started using it yesterday and works perfectly!

RustyF · 2019-04-08T09:44:17Z

Thanks for the feedback - good to know

ghost · 2019-04-21T13:50:20Z

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!

RustyF added 2 commits February 6, 2019 00:05

WIP initial schema

0b2658a

Fleshed out implementation and adjusted schema to align with azure AP…

188b3ac

…I docs

ghost added the size/L label Feb 18, 2019

Documentation draft

7f16cdc

ghost added the documentation label Feb 18, 2019

Test WIP

a74ff57

ghost added size/XL and removed size/L labels Feb 18, 2019

RustyF added 4 commits February 18, 2019 22:12

Ensure we can name redirect from routing rule

b299e85

First stab at test for target httpListener scenario

2979ffa

Missing computed ID properties (finally understand how they work)

ddbf0f4

Mis-spelt variable

1ff625c

katbyte reviewed Feb 20, 2019

View reviewed changes

azurerm/resource_arm_application_gateway.go Outdated Show resolved Hide resolved

azurerm/resource_arm_application_gateway.go Show resolved Hide resolved

azurerm/resource_arm_application_gateway.go Show resolved Hide resolved

katbyte added enhancement service/application-gateway labels Feb 20, 2019

RustyF added 2 commits February 24, 2019 21:44

Use Go Azure SDK constants

a9ca44d

Declare some mutual exclusions using ConflictsWith...

91abb6e

katbyte reviewed Feb 26, 2019

View reviewed changes

azurerm/resource_arm_application_gateway.go Outdated Show resolved Hide resolved

RustyF added 4 commits February 27, 2019 23:01

Ensure http_listener optional now (in case of redirection)

cbdd835

Remove conflicts (not usable for nested resources) Add validation in expansion funcs

Correction - HTTPListener required on routing rule

db7aef2

tests for basic and path-based routing rules with redirection in plac…

2fc242e

…e (with target listener and target URL mix) (not run yet - local environment issues)

Validate for no empty strings

5d18d1e

tombuildsstuff suggested changes Feb 28, 2019

View reviewed changes

tombuildsstuff changed the title ~~Redirect configuration~~ azurerm_application_gateway - support for redirect rules Feb 28, 2019

tombuildsstuff added the waiting-response label Mar 1, 2019

RustyF added 2 commits March 3, 2019 16:51

Introduce NilOrEmpty() validation to correctly check strings, i.e. Ta…

32cdb89

…rgetURL is fully specified. Not sure yet what to do with mutually-exclusive SubResource * references.

Fixed bug where wrong local was referenced.

be131cf

ghost removed the waiting-response label Mar 4, 2019

Ensure that optional redirect configuration properties are only set i…

6a79696

…f specified Ensure that default HTTP settings and back-end pool settings are now optional on path URL map (to facilitate redirections) Tweak tests to reflect better understanding of path URL map

Some missing parameters for redirect_configuration use. Plus some pre…

57e73e7

…viously required properties are now optional.

katbyte requested changes Mar 13, 2019

View reviewed changes

azurerm/helpers/validate/strings.go Outdated Show resolved Hide resolved

azurerm/resource_arm_application_gateway.go Outdated Show resolved Hide resolved

RustyF added 3 commits March 17, 2019 14:57

Drop nilOrEmpty() on the basis that it's hardly providing much value/…

0d74b8f

…abstraction

goimports

4598d22

Merge remote-tracking branch 'remotes/upstream/master' into redirect-…

0808922

…rules

katbyte previously requested changes Mar 21, 2019

View reviewed changes

katbyte added the waiting-response label Mar 21, 2019

katbyte approved these changes Mar 21, 2019

View reviewed changes

Merge branch 'master' into redirect-rules

dfa5ca6

katbyte added this to the v1.24.0 milestone Mar 21, 2019

katbyte merged commit 0846e21 into hashicorp:master Mar 21, 2019

katbyte added a commit that referenced this pull request Mar 21, 2019

Update CHANGELOG.md to include #2908

9811618

ghost removed the waiting-response label Mar 22, 2019

jstewart612 mentioned this pull request Mar 25, 2019

Bug Fixes/Enhancements to Application Gateway #1576

Closed

ghost locked and limited conversation to collaborators Apr 21, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`azurerm_application_gateway` - support for redirect rules #2908

`azurerm_application_gateway` - support for redirect rules #2908

RustyF commented Feb 18, 2019 •

edited

Loading

katbyte left a comment

RustyF commented Feb 21, 2019

katbyte commented Feb 22, 2019

RustyF commented Feb 28, 2019

tombuildsstuff left a comment

RustyF commented Mar 5, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 9, 2019

katbyte left a comment

RustyF commented Mar 17, 2019

katbyte left a comment •

edited

Loading

katbyte left a comment

RustyF commented Mar 22, 2019

ghost commented Apr 3, 2019

OffColour commented Apr 8, 2019

RustyF commented Apr 8, 2019

ghost commented Apr 21, 2019

azurerm_application_gateway - support for redirect rules #2908

azurerm_application_gateway - support for redirect rules #2908

Conversation

RustyF commented Feb 18, 2019 • edited Loading

katbyte left a comment

Choose a reason for hiding this comment

RustyF commented Feb 21, 2019

katbyte commented Feb 22, 2019

RustyF commented Feb 28, 2019

tombuildsstuff left a comment

Choose a reason for hiding this comment

RustyF commented Mar 5, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 6, 2019

RustyF commented Mar 9, 2019

katbyte left a comment

Choose a reason for hiding this comment

RustyF commented Mar 17, 2019

katbyte left a comment • edited Loading

Choose a reason for hiding this comment

katbyte left a comment

Choose a reason for hiding this comment

RustyF commented Mar 22, 2019

ghost commented Apr 3, 2019

OffColour commented Apr 8, 2019

RustyF commented Apr 8, 2019

ghost commented Apr 21, 2019

`azurerm_application_gateway` - support for redirect rules #2908

`azurerm_application_gateway` - support for redirect rules #2908

RustyF commented Feb 18, 2019 •

edited

Loading

katbyte left a comment •

edited

Loading