-
Notifications
You must be signed in to change notification settings - Fork 85
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
Implement DFW settings Exclusion list #1037
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
118 changes: 118 additions & 0 deletions
118
api/infra/settings/firewall/security/policy_exclude_list.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,118 @@ | ||
//nolint:revive | ||
package security | ||
|
||
// The following file has been autogenerated. Please avoid any changes! | ||
import ( | ||
"errors" | ||
|
||
vapiProtocolClient_ "github.com/vmware/vsphere-automation-sdk-go/runtime/protocol/client" | ||
client1 "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/global_infra/settings/firewall/security" | ||
model1 "github.com/vmware/vsphere-automation-sdk-go/services/nsxt-gm/model" | ||
client0 "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/infra/settings/firewall/security" | ||
model0 "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" | ||
|
||
utl "github.com/vmware/terraform-provider-nsxt/api/utl" | ||
) | ||
|
||
type PolicyExcludeListClientContext utl.ClientContext | ||
|
||
func NewExcludeListClient(sessionContext utl.SessionContext, connector vapiProtocolClient_.Connector) *PolicyExcludeListClientContext { | ||
var client interface{} | ||
|
||
switch sessionContext.ClientType { | ||
|
||
case utl.Local: | ||
client = client0.NewExcludeListClient(connector) | ||
|
||
case utl.Global: | ||
client = client1.NewExcludeListClient(connector) | ||
|
||
default: | ||
return nil | ||
} | ||
return &PolicyExcludeListClientContext{Client: client, ClientType: sessionContext.ClientType, ProjectID: sessionContext.ProjectID} | ||
} | ||
|
||
func (c PolicyExcludeListClientContext) Get() (model0.PolicyExcludeList, error) { | ||
var obj model0.PolicyExcludeList | ||
var err error | ||
|
||
switch c.ClientType { | ||
|
||
case utl.Local: | ||
client := c.Client.(client0.ExcludeListClient) | ||
obj, err = client.Get() | ||
if err != nil { | ||
return obj, err | ||
} | ||
|
||
case utl.Global: | ||
client := c.Client.(client1.ExcludeListClient) | ||
gmObj, err1 := client.Get() | ||
if err1 != nil { | ||
return obj, err1 | ||
} | ||
var rawObj interface{} | ||
rawObj, err = utl.ConvertModelBindingType(gmObj, model1.PolicyExcludeListBindingType(), model0.PolicyExcludeListBindingType()) | ||
obj = rawObj.(model0.PolicyExcludeList) | ||
|
||
default: | ||
return obj, errors.New("invalid infrastructure for model") | ||
} | ||
return obj, err | ||
} | ||
|
||
func (c PolicyExcludeListClientContext) Patch(policyExcludeListParam model0.PolicyExcludeList) error { | ||
var err error | ||
|
||
switch c.ClientType { | ||
|
||
case utl.Local: | ||
client := c.Client.(client0.ExcludeListClient) | ||
err = client.Patch(policyExcludeListParam) | ||
|
||
case utl.Global: | ||
client := c.Client.(client1.ExcludeListClient) | ||
gmObj, err1 := utl.ConvertModelBindingType(policyExcludeListParam, model0.PolicyExcludeListBindingType(), model1.PolicyExcludeListBindingType()) | ||
if err1 != nil { | ||
return err1 | ||
} | ||
err = client.Patch(gmObj.(model1.PolicyExcludeList)) | ||
|
||
default: | ||
err = errors.New("invalid infrastructure for model") | ||
} | ||
return err | ||
} | ||
|
||
func (c PolicyExcludeListClientContext) Update(policyExcludeListParam model0.PolicyExcludeList) (model0.PolicyExcludeList, error) { | ||
var err error | ||
var obj model0.PolicyExcludeList | ||
|
||
switch c.ClientType { | ||
|
||
case utl.Local: | ||
client := c.Client.(client0.ExcludeListClient) | ||
obj, err = client.Update(policyExcludeListParam) | ||
|
||
case utl.Global: | ||
client := c.Client.(client1.ExcludeListClient) | ||
gmObj, err := utl.ConvertModelBindingType(policyExcludeListParam, model0.PolicyExcludeListBindingType(), model1.PolicyExcludeListBindingType()) | ||
if err != nil { | ||
return obj, err | ||
} | ||
gmObj, err = client.Update(gmObj.(model1.PolicyExcludeList)) | ||
if err != nil { | ||
return obj, err | ||
} | ||
obj1, err1 := utl.ConvertModelBindingType(gmObj, model1.PolicyExcludeListBindingType(), model0.PolicyExcludeListBindingType()) | ||
if err1 != nil { | ||
return obj, err1 | ||
} | ||
obj = obj1.(model0.PolicyExcludeList) | ||
|
||
default: | ||
err = errors.New("invalid infrastructure for model") | ||
} | ||
return obj, err | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
144 changes: 144 additions & 0 deletions
144
nsxt/resource_nsxt_policy_firewall_exclude_list_member.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,144 @@ | ||
/* Copyright © 2023 VMware, Inc. All Rights Reserved. | ||
SPDX-License-Identifier: MPL-2.0 */ | ||
|
||
package nsxt | ||
|
||
import ( | ||
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" | ||
utl "github.com/vmware/terraform-provider-nsxt/api/utl" | ||
"github.com/vmware/vsphere-automation-sdk-go/lib/vapi/std/errors" | ||
"github.com/vmware/vsphere-automation-sdk-go/runtime/protocol/client" | ||
"github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" | ||
|
||
"github.com/vmware/terraform-provider-nsxt/api/infra/settings/firewall/security" | ||
) | ||
|
||
func resourceNsxtPolicyFirewallExcludeListMember() *schema.Resource { | ||
return &schema.Resource{ | ||
Create: resourceNsxtPolicyFirewallExcludeListMemberCreate, | ||
Read: resourceNsxtPolicyFirewallExcludeListMemberRead, | ||
Delete: resourceNsxtPolicyFirewallExcludeListMemberDelete, | ||
Importer: &schema.ResourceImporter{ | ||
State: schema.ImportStatePassthrough, | ||
}, | ||
Schema: map[string]*schema.Schema{ | ||
"member": { | ||
Type: schema.TypeString, | ||
Description: "ExcludeList member", | ||
Required: true, | ||
ForceNew: true, | ||
ValidateFunc: validatePolicyPath(), | ||
}, | ||
}, | ||
} | ||
} | ||
|
||
func memberInList(member string, members []string) int { | ||
for i, mem := range members { | ||
if mem == member { | ||
return i | ||
} | ||
} | ||
return -1 | ||
} | ||
|
||
func resourceNsxtPolicyFirewallExcludeListMemberExists(sessionContext utl.SessionContext, id string, connector client.Connector) (bool, error) { | ||
|
||
client := security.NewExcludeListClient(sessionContext, connector) | ||
obj, err := client.Get() | ||
if isNotFoundError(err) { | ||
return false, nil | ||
} else if err != nil { | ||
return false, err | ||
} | ||
if 0 <= memberInList(id, obj.Members) { | ||
return true, nil | ||
} | ||
|
||
return false, nil | ||
} | ||
|
||
func resourceNsxtPolicyFirewallExcludeListMemberCreate(d *schema.ResourceData, m interface{}) error { | ||
connector := getPolicyConnector(m) | ||
member := d.Get("member").(string) | ||
|
||
doUpdate := func() error { | ||
var obj model.PolicyExcludeList | ||
|
||
client := security.NewExcludeListClient(getSessionContext(d, m), connector) | ||
obj, err := client.Get() | ||
if isNotFoundError(err) { | ||
obj = model.PolicyExcludeList{ | ||
Members: []string{member}, | ||
} | ||
} else if err != nil { | ||
return err | ||
} | ||
if 0 <= memberInList(member, obj.Members) { | ||
return errors.AlreadyExists{} | ||
} | ||
obj.Members = append(obj.Members, member) | ||
_, err = client.Update(obj) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
d.SetId(member) | ||
|
||
return nil | ||
} | ||
commonProviderConfig := getCommonProviderConfig(m) | ||
err := retryUponPreconditionFailed(doUpdate, commonProviderConfig.MaxRetries) | ||
if err != nil { | ||
return handleCreateError("PolicyFirewallExcludeListMember", member, err) | ||
} | ||
|
||
return resourceNsxtPolicyFirewallExcludeListMemberRead(d, m) | ||
} | ||
|
||
func resourceNsxtPolicyFirewallExcludeListMemberRead(d *schema.ResourceData, m interface{}) error { | ||
connector := getPolicyConnector(m) | ||
member := d.Id() | ||
|
||
client := security.NewExcludeListClient(getSessionContext(d, m), connector) | ||
obj, err := client.Get() | ||
if err != nil { | ||
return handleReadError(d, "PolicyFirewallExcludeListMember", member, err) | ||
} | ||
if 0 > memberInList(member, obj.Members) { | ||
return errors.NotFound{} | ||
} | ||
d.Set("member", member) | ||
return nil | ||
} | ||
|
||
func resourceNsxtPolicyFirewallExcludeListMemberDelete(d *schema.ResourceData, m interface{}) error { | ||
connector := getPolicyConnector(m) | ||
member := d.Get("member").(string) | ||
|
||
doUpdate := func() error { | ||
var obj model.PolicyExcludeList | ||
|
||
client := security.NewExcludeListClient(getSessionContext(d, m), connector) | ||
obj, err := client.Get() | ||
if isNotFoundError(err) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should silently swallow the error in |
||
return nil | ||
} else if err != nil { | ||
return err | ||
} | ||
i := memberInList(member, obj.Members) | ||
if i < 0 { | ||
return errors.NotFound{} | ||
} | ||
|
||
obj.Members = append(obj.Members[:i], obj.Members[i+1:]...) | ||
_, err = client.Update(obj) | ||
return err | ||
} | ||
commonProviderConfig := getCommonProviderConfig(m) | ||
err := retryUponPreconditionFailed(doUpdate, commonProviderConfig.MaxRetries) | ||
if err != nil { | ||
return handleDeleteError("PolicyFirewallExcludeListMember", member, err) | ||
} | ||
return nil | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make more sense to implement the list as one resource (with list of paths inside?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've considered - But then we'll run into the usual issue where this singleton NSX object is virtually indestructible using NSX API - TF doesn't really like these, I think. Is there any decent way to work around this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can revert to default (empty list in this case?) when resource is destroyed - we do the same for vm tags, for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can - the question is if there is any advantage in handling this as a scalar, vs handling as a list which is more "natural" to terraform with full CRUD operation.
We've done this for context profile custom attributes and such.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My personal feeling is that a single string does not justify a dedicated resource. But I don't insist.
As long as we're sure order is not important in this list, we can do it as suggested here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annakhm there is a more serious problem which I should look into:
When two members are created at the same time, there is a concurrency problem - both retrieve the member list, append the new member and when submitting one change is overwritten.
I'm not sure if we have some mean to handle this (and we might have this elsewhere maybe).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I worked around the concurrency problem, and added a few test. I'd rather if you could have another look...