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

Adding IP restrictions to Web App #1231

Merged
merged 6 commits into from
May 14, 2018

Conversation

paktek123
Copy link
Contributor

This is fix for issue #1157

Copy link
Contributor

@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 @paktek123

Thanks for this PR :)

I've taken a look through and left some comments inline - since this PR is so close, I hope you don't mind but I've pushed a couple of commits to fix the issues I've raised - and this now LGTM 👍

Thanks!

"subnet_mask": {
Type: schema.TypeString,
Optional: true,
Default: "255.255.255.255",
Copy link
Contributor

Choose a reason for hiding this comment

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

context from the Azure Portal: Specify an IP address range by providing a subnet mask (i.e. 255.255.255.0). If no mask is provided 255.255.255.255 is assumed.


ip_restriction {
ip_address = "10.10.10.10"
subnet_mask = "255.255.255.255"
Copy link
Contributor

Choose a reason for hiding this comment

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

since it's defaulted, we can remove this value


ip_restriction {
ip_address = "10.10.10.10"
subnet_mask = "255.255.255.255"
Copy link
Contributor

Choose a reason for hiding this comment

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

(same here)

ipSecurityRestrictions := configResp.SiteConfig.IPSecurityRestrictions
if ipSecurityRestrictions != nil {
d.Set("ip_restrictions", flattenAppServiceIpRestrictions(ipSecurityRestrictions))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should set this all the time (but return an empty list) - I'll push a commit to fix this

ipSecurityRestrictions := configResp.SiteConfig.IPSecurityRestrictions
if ipSecurityRestrictions != nil {
d.Set("ip_restriction", flattenAppServiceIpRestrictions(ipSecurityRestrictions))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

we should set this all the time (but return an empty list) - I'll push a commit to fix this

@@ -123,6 +123,23 @@ func dataSourceArmAppService() *schema.Resource {
Computed: true,
},

"ip_restriction": {
Copy link
Contributor

Choose a reason for hiding this comment

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

can we add some additional documentation for these fields?

@tombuildsstuff tombuildsstuff added this to the 1.6.0 milestone May 14, 2018
@tombuildsstuff
Copy link
Contributor

Ignoring a temporary connection error; the resource tests pass:

screen shot 2018-05-14 at 14 22 01

Also the Data Source tests pass:

screen shot 2018-05-14 at 14 16 59

@tombuildsstuff tombuildsstuff merged commit 26834a2 into hashicorp:master May 14, 2018
tombuildsstuff added a commit that referenced this pull request May 14, 2018
@paktek123
Copy link
Contributor Author

Thanks @tombuildsstuff !

@katbyte
Copy link
Collaborator

katbyte commented May 25, 2018

Hey @cori,

Just wanted to let you know we have released v1.6.0 of the provider allowing for IP restrictions 🙂

@ghost
Copy link

ghost commented Mar 31, 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 👉 [email protected]. Thanks!

@ghost ghost locked and limited conversation to collaborators Mar 31, 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.

3 participants