Lb nat pool #24
Lb nat pool #24
Conversation
There was a problem hiding this comment.
Hi @thetonymaster,
Thank you for the PR, I have left some comments inline that need to be addressed.
I have applied these changes to the azure provider here
| return &schema.Resource{ | ||
| Create: resourceArmLoadBalancerNatPoolCreate, | ||
| Read: resourceArmLoadBalancerNatPoolRead, | ||
| Update: resourceArmLoadBalancerNatPoolCreate, |
There was a problem hiding this comment.
Could we change this to resourceArmLoadBalancerNatPoolCreateUpdate ?
| Type: schema.TypeString, | ||
| Required: true, | ||
| ForceNew: true, | ||
| }, |
There was a problem hiding this comment.
Could we get some validation here? at a minimum validation.NoZeroValues
| Required: true, | ||
| StateFunc: ignoreCaseStateFunc, | ||
| DiffSuppressFunc: ignoreCaseDiffSuppressFunc, | ||
| }, |
There was a problem hiding this comment.
Could we add validation for all possible protocols here?
| "frontend_port_end": { | ||
| Type: schema.TypeInt, | ||
| Required: true, | ||
| }, |
There was a problem hiding this comment.
Could we validate this and ensure the values is a valid port number?
| "backend_port": { | ||
| Type: schema.TypeInt, | ||
| Required: true, | ||
| }, |
There was a problem hiding this comment.
Could we validate this and ensure the values is a valid port number?
| ID: rule.ID, | ||
| } | ||
|
|
||
| properties.FrontendIPConfiguration = &feip |
There was a problem hiding this comment.
This could become
properties.FrontendIPConfiguration = &network.SubResource{
ID: rule.ID,
}| InboundNatPoolPropertiesFormat: &properties, | ||
| } | ||
|
|
||
| return &natPool, nil |
There was a problem hiding this comment.
this could become
return &network.InboundNatPool{
Name: utils.String(d.Get("name").(string)),
InboundNatPoolPropertiesFormat: &properties,
}, nil| return fmt.Sprintf(` | ||
|
|
||
| resource "azurestack_resource_group" "test" { | ||
| name = "acctestRG-%d" |
There was a problem hiding this comment.
Very minor but could we align the =s here?
| page_title: "Azure Resource Manager: azurestack_lb_nat_pool" | ||
| sidebar_current: "docs-azurestack-resource-loadbalancer-nat-pool" | ||
| description: |- | ||
| Create a LoadBalancer NAT Pool. |
There was a problem hiding this comment.
Could we change this to Manages?
There was a problem hiding this comment.
Also LoadBalancer -> Load Balancer to match the azure docs
|
|
||
| # azurestack_lb_nat_pool | ||
|
|
||
| Create a LoadBalancer NAT pool. |
There was a problem hiding this comment.
Could we change this to Manages?
There was a problem hiding this comment.
Also LoadBalancer -> Load Balancer to match the azure docs (all instances of this should be fixed)
|
tests pass: |
|
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
locked and limited conversation to collaborators
No description provided.