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

Add utility to parse standard policy path #1218

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ require (
github.com/google/uuid v1.3.0
github.com/hashicorp/go-version v1.6.0
github.com/hashicorp/terraform-plugin-sdk/v2 v2.29.0
github.com/stretchr/testify v1.7.2
github.com/vmware/go-vmware-nsxt v0.0.0-20220328155605-f49a14c1ef5f
github.com/vmware/vsphere-automation-sdk-go/lib v0.7.0
github.com/vmware/vsphere-automation-sdk-go/runtime v0.7.0
Expand All @@ -22,6 +23,7 @@ require (
github.com/apparentlymart/go-textseg/v15 v15.0.0 // indirect
github.com/beevik/etree v1.1.0 // indirect
github.com/cloudflare/circl v1.3.7 // indirect
github.com/davecgh/go-spew v1.1.1 // indirect
github.com/fatih/color v1.13.0 // indirect
github.com/gibson042/canonicaljson-go v1.0.3 // indirect
github.com/golang-jwt/jwt/v4 v4.3.0 // indirect
Expand Down Expand Up @@ -53,6 +55,7 @@ require (
github.com/mitchellh/mapstructure v1.5.0 // indirect
github.com/mitchellh/reflectwalk v1.0.2 // indirect
github.com/oklog/run v1.0.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
github.com/vmihailenco/msgpack/v5 v5.3.5 // indirect
github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
Expand All @@ -67,4 +70,5 @@ require (
google.golang.org/genproto/googleapis/rpc v0.0.0-20230525234030-28d5490b6b19 // indirect
google.golang.org/grpc v1.57.1 // indirect
google.golang.org/protobuf v1.33.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
)
84 changes: 84 additions & 0 deletions nsxt/policy_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,29 @@ func getOrGenerateID(d *schema.ResourceData, m interface{}, presenceChecker func
return id, nil
}

//lint:ignore U1000 Ignore unused function temporarily until used in autogenerated resource
func getOrGenerateIDWithParent(d *schema.ResourceData, m interface{}, presenceChecker func(utl.SessionContext, string, string, client.Connector) (bool, error)) (string, error) {
connector := getPolicyConnector(m)

id := d.Get("nsx_id").(string)
if id == "" {
return newUUID(), nil
}

parentPath := d.Get("parent_path").(string)

exists, err := presenceChecker(getSessionContext(d, m), parentPath, id, connector)
if err != nil {
return "", err
}

if exists {
return "", fmt.Errorf("Resource with id %s already exists", id)
}

return id, nil
}

func newUUID() string {
uuid, _ := uuid.NewRandom()
return uuid.String()
Expand Down Expand Up @@ -188,6 +211,67 @@ func getResourceIDFromResourcePath(rPath string, rType string) string {
return ""
}

func parseStandardPolicyPath(path string) ([]string, error) {
Copy link
Member

Choose a reason for hiding this comment

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

Do you just need to collect the list of identifiers in the policy path, or will it also matter figuring out what is what?

For instance in:
/orgs/myorg/projects/myproj/infra/tier-1s/mygw1"
parent[2] is a router id

but in:
/orgs/myorg/projects/myproj/vpcs/myvpc/tier-1s/mygw1
parent[2] is a VPC id

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Parent list will be used as parameters for API calls, so in case of orgs/projects/vpc the ids will be used as parameters in API call in the same way as gw id or segment id would be used.

var parents []string
segments := strings.Split(path, "/")
if len(segments) < 3 {
return nil, fmt.Errorf("invalid policy path %s", path)
}
if segments[0] != "" {
return nil, fmt.Errorf("policy path is expected to start with /")
}
// starting with *infra index
idx := 1
infraPath := true
if segments[1] == "orgs" {
if len(segments) < 5 {
return nil, fmt.Errorf("invalid multitenant policy path %s", path)
}

// append org and project
parents = append(parents, segments[2])
parents = append(parents, segments[4])
idx = 5

if len(segments) > 6 && segments[5] == "vpcs" {
parents = append(parents, segments[6])
idx = 7
// vpc paths do not contain infra
infraPath = false
}
}
if len(segments) <= idx {
return nil, fmt.Errorf("unexpected policy path %s", path)
}
if infraPath {
// continue after infra marker
if segments[idx] != "infra" && segments[idx] != "global-infra" {
Copy link
Member

Choose a reason for hiding this comment

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

I don't remember a case where we can have global-infra for a project-scoped resource.
At the same time, I don't think that a special case to handle only "infra" when in org scope is mandatory

return nil, fmt.Errorf("policy path %s is expected to contain *infra marker", path)
}
idx++
}

for i, seg := range segments[idx:] {
// in standard policy path, odd segments are object ids
if i%2 == 1 {
parents = append(parents, seg)
}
}
return parents, nil
}

func parseStandardPolicyPathVerifySize(path string, expectedSize int) ([]string, error) {
parents, err := parseStandardPolicyPath(path)
if err != nil {
return parents, err
}
if len(parents) != expectedSize {
return parents, fmt.Errorf("Unexpected parent path %s (expected %d parent ids, got %d)", path, expectedSize, len(parents))
}

return parents, nil
}

func nsxtDomainResourceImporter(d *schema.ResourceData, m interface{}) ([]*schema.ResourceData, error) {
importDomain := defaultDomain
importID := d.Id()
Expand Down
101 changes: 101 additions & 0 deletions nsxt/policy_utils_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/* Copyright © 2024 Broadcom, Inc. All Rights Reserved.
SPDX-License-Identifier: MPL-2.0 */

package nsxt

import (
"testing"

"github.com/stretchr/testify/assert"
)

type policyPathTest struct {
path string
parents []string
}

func TestParseStandardPolicyPath(t *testing.T) {

testData := []policyPathTest{
Copy link
Member

Choose a reason for hiding this comment

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

nit: add negative test with invalid path and verify code fails as expected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is a negative test below

{
path: "/infra/tier-1s/mygw1",
parents: []string{"mygw1"},
},
{
path: "/global-infra/tier-0s/mygw1",
parents: []string{"mygw1"},
},
{
path: "/orgs/myorg/projects/myproj/infra/tier-1s/mygw1",
parents: []string{"myorg", "myproj", "mygw1"},
},
{
path: "/orgs/myorg/projects/myproj/vpcs/myvpc/tier-1s/mygw1",
parents: []string{"myorg", "myproj", "myvpc", "mygw1"},
},
{
path: "/orgs/myorg/projects/myproj/infra/domains/d1/groups/g1",
parents: []string{"myorg", "myproj", "d1", "g1"},
},
{
path: "/global-infra/tier-1s/t15/ipsec-vpn-services/default/sessions/xxx-yyy-xxx",
parents: []string{"t15", "default", "xxx-yyy-xxx"},
},
{
path: "/infra/evpn-tenant-configs/{config-id}",
parents: []string{"{config-id}"},
},
{
path: "/infra/tier-0s/{tier-0-id}/locale-services/{locale-service-id}/service-interfaces/{interface-id}",
salv-orlando marked this conversation as resolved.
Show resolved Hide resolved
parents: []string{"{tier-0-id}", "{locale-service-id}", "{interface-id}"},
},
}

for _, test := range testData {
parents, err := parseStandardPolicyPath(test.path)
assert.Nil(t, err)
assert.Equal(t, test.parents, parents)
}
}

func TestIsPolicyPath(t *testing.T) {

testData := []string{
"/infra/tier-1s/mygw1",
"/global-infra/tier-1s/mygw1",
"/orgs/infra/tier-1s/mygw1",
"/orgs/myorg/projects/myproj/domains/d",
}

for _, test := range testData {
pp := isPolicyPath(test)
assert.True(t, pp)
}
}

func TestNegativeParseStandardPolicyPath(t *testing.T) {

testData := []string{
"/some-infra/tier-1s/mygw1",
"orgs/infra/tier-1s/mygw1-1",
"orgs/infra /tier-1s/mygw1-1",
}

for _, test := range testData {
_, err := parseStandardPolicyPath(test)
assert.NotNil(t, err)
}
}

func TestParseStandardPolicyPathVerifySize(t *testing.T) {

_, err := parseStandardPolicyPathVerifySize("/infra/things/thing1/sub-things/sub-thing1", 3)
assert.NotNil(t, err)

parents, err := parseStandardPolicyPathVerifySize("/infra/things/thing1/sub-things/sub-thing1", 2)
assert.Nil(t, err)
assert.Equal(t, 2, len(parents))

_, err = parseStandardPolicyPathVerifySize("/global-infra/things/1/sub-things/2/fine-tuned-thing/3", 1)
assert.NotNil(t, err)
}
Loading