fix: filter building (#17)
- update filter building for `dryRunValidation` to append with `AND` - Add test coverage for filter building and dryRunValidation
This commit is contained in:
Elim Tsiagbey authored and 
GitHub committed
1 parent
0d5cc5d444
commit
7076cd0bad
3 files changed
+309
-4
No files matched your search
@@ -6,6 +6,7 @@ package host
|
||||
import (
|
||||
"testing"
|
||||
|
||||
computepb "cloud.google.com/go/compute/apiv1/computepb"
|
||||
cred "github.com/hashicorp/boundary-plugin-gcp/internal/credential"
|
||||
"github.com/stretchr/testify/require"
|
||||
"google.golang.org/grpc/codes"
|
||||
@@ -126,3 +127,82 @@ func TestGetSetAttributes(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildListInstancesRequest(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
attributes *SetAttributes
|
||||
catalog *CatalogAttributes
|
||||
expected *computepb.ListInstancesRequest
|
||||
expectedErrContains string
|
||||
}{
|
||||
{
|
||||
name: "valid request with single filter",
|
||||
attributes: &SetAttributes{
|
||||
Filters: []string{"tags.items=my-tag"},
|
||||
},
|
||||
catalog: &CatalogAttributes{
|
||||
CredentialAttributes: &cred.CredentialAttributes{
|
||||
ProjectId: "test-12345",
|
||||
Zone: "us-central-1a",
|
||||
},
|
||||
},
|
||||
expected: &computepb.ListInstancesRequest{
|
||||
Project: "test-12345",
|
||||
Zone: "us-central-1a",
|
||||
Filter: stringPtr("tags.items=my-tag AND status = running"),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "valid request with multiple filters",
|
||||
attributes: &SetAttributes{
|
||||
Filters: []string{"tags.items=my-tag", "tags.items=my-other-tag"},
|
||||
},
|
||||
catalog: &CatalogAttributes{
|
||||
CredentialAttributes: &cred.CredentialAttributes{
|
||||
ProjectId: "test-12345",
|
||||
Zone: "us-central-1a",
|
||||
},
|
||||
},
|
||||
expected: &computepb.ListInstancesRequest{
|
||||
Project: "test-12345",
|
||||
Zone: "us-central-1a",
|
||||
Filter: stringPtr("tags.items=my-tag AND tags.items=my-other-tag AND status = running"),
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "error building filters",
|
||||
attributes: &SetAttributes{
|
||||
Filters: []string{"invalid-filter"},
|
||||
},
|
||||
catalog: &CatalogAttributes{
|
||||
CredentialAttributes: &cred.CredentialAttributes{
|
||||
ProjectId: "test-12345",
|
||||
Zone: "us-central-1a",
|
||||
},
|
||||
},
|
||||
expectedErrContains: "error building filters",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
actual, err := buildListInstancesRequest(tc.attributes, tc.catalog)
|
||||
if tc.expectedErrContains != "" {
|
||||
require.Error(err)
|
||||
require.Contains(err.Error(), tc.expectedErrContains)
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(err)
|
||||
require.Equal(tc.expected, actual)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func stringPtr(s string) *string {
|
||||
return &s
|
||||
}
|
||||
@@ -624,7 +624,7 @@ func dryRunValidation(
|
||||
|
||||
var filters string
|
||||
if len(listFilters) > 0 {
|
||||
filters = strings.Join(listFilters, " ")
|
||||
filters = strings.Join(listFilters, " AND ")
|
||||
}
|
||||
|
||||
it := instancesClient.List(ctx, &computepb.ListInstancesRequest{
|
||||
@@ -649,10 +649,10 @@ func buildFilters(attrs *SetAttributes) ([]string, error) {
|
||||
// If filters are provided, validate them
|
||||
if len(attrs.Filters) != 0 {
|
||||
for _, filterAttr := range attrs.Filters {
|
||||
key, operator, value, valid := extractFilterValue(filterAttr)
|
||||
key, _, value, valid := extractFilterValue(filterAttr)
|
||||
switch {
|
||||
case !valid:
|
||||
return nil, fmt.Errorf("filter %q contains invalid operator: %s. Use one of =, !=, >, <, <=, >=, :, eq, ne", filterAttr, operator)
|
||||
return nil, fmt.Errorf("invalid filter %q. Ensure the operator is one of =, !=, >, <, <=, >=, :, eq, ne", filterAttr)
|
||||
case len(strings.TrimSpace(key)) == 0:
|
||||
return nil, fmt.Errorf("filter %q contains an empty filter key", filterAttr)
|
||||
case len(strings.TrimSpace(value)) == 0:
|
||||
|
||||
@@ -663,7 +663,7 @@ func TestListHosts(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
expectedErr: "error building filters: filter \"not-a-filter\" contains invalid operator",
|
||||
expectedErr: "error building filters: invalid filter \"not-a-filter\"",
|
||||
expectedErrCode: codes.InvalidArgument,
|
||||
},
|
||||
}
|
||||
@@ -2191,3 +2191,228 @@ func TestNormalizeSetData(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestBuildFilters(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
attrs *SetAttributes
|
||||
expected []string
|
||||
expectedErr string
|
||||
}{
|
||||
{
|
||||
name: "no filters provided",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{},
|
||||
},
|
||||
expected: []string{defaultStatusFilter},
|
||||
},
|
||||
{
|
||||
name: "valid filters provided",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{"name = instance-1", "zone = us-central1-a"},
|
||||
},
|
||||
expected: []string{"name = instance-1", "zone = us-central1-a", defaultStatusFilter},
|
||||
},
|
||||
{
|
||||
name: "filter with invalid operator",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{"name ~ instance-1"},
|
||||
},
|
||||
expectedErr: "invalid filter \"name ~ instance-1\"",
|
||||
},
|
||||
{
|
||||
name: "filter with empty key",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{" = instance-1"},
|
||||
},
|
||||
expectedErr: "invalid filter \" = instance-1\"",
|
||||
},
|
||||
{
|
||||
name: "filter with empty value",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{"name = "},
|
||||
},
|
||||
expectedErr: "filter \"name = \" contains an empty value",
|
||||
},
|
||||
{
|
||||
name: "filter with status key",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{"status = terminated"},
|
||||
},
|
||||
expected: []string{"status = terminated"},
|
||||
},
|
||||
{
|
||||
name: "filter with multiple filters",
|
||||
attrs: &SetAttributes{
|
||||
Filters: []string{"status = terminated", "name = instance-1"},
|
||||
},
|
||||
expected: []string{"status = terminated", "name = instance-1"},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
actual, err := buildFilters(tc.attrs)
|
||||
if tc.expectedErr != "" {
|
||||
require.Contains(err.Error(), tc.expectedErr)
|
||||
return
|
||||
}
|
||||
require.NoError(err)
|
||||
require.Equal(tc.expected, actual)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestDryRunValidation(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
cases := []struct {
|
||||
name string
|
||||
state *gcpCatalogPersistedState
|
||||
listFilters []string
|
||||
clientOptions []option.ClientOption
|
||||
expectedErr string
|
||||
expectedErrCode codes.Code
|
||||
}{
|
||||
{
|
||||
name: "nil state",
|
||||
state: nil,
|
||||
expectedErr: "persisted state is required",
|
||||
expectedErrCode: codes.InvalidArgument,
|
||||
},
|
||||
{
|
||||
name: "error getting instances client",
|
||||
state: &gcpCatalogPersistedState{
|
||||
PersistedState: &cred.PersistedState{
|
||||
CredentialsConfig: &cred.Config{
|
||||
ProjectId: "test-project",
|
||||
Zone: "us-central1-a",
|
||||
},
|
||||
},
|
||||
testInstancesAPIFunc: newTestMockInstances(ctx,
|
||||
nil,
|
||||
testMockInstancesWithListInstancesOutput(nil),
|
||||
testMockInstancesWithListInstancesError(fmt.Errorf("failed to list instances")),
|
||||
),
|
||||
},
|
||||
clientOptions: []option.ClientOption{
|
||||
option.WithHTTPClient(&http.Client{}),
|
||||
},
|
||||
expectedErr: "gcp list instances failed",
|
||||
expectedErrCode: codes.FailedPrecondition,
|
||||
},
|
||||
{
|
||||
name: "list instances error",
|
||||
state: &gcpCatalogPersistedState{
|
||||
PersistedState: &cred.PersistedState{
|
||||
CredentialsConfig: &cred.Config{
|
||||
ProjectId: "test-project",
|
||||
Zone: "us-central1-a",
|
||||
},
|
||||
},
|
||||
testInstancesAPIFunc: newTestMockInstances(ctx,
|
||||
nil,
|
||||
testMockInstancesWithListInstancesOutput(nil),
|
||||
testMockInstancesWithListInstancesError(fmt.Errorf("failed to list instances")),
|
||||
),
|
||||
},
|
||||
expectedErr: "gcp list instances failed",
|
||||
expectedErrCode: codes.FailedPrecondition,
|
||||
},
|
||||
{
|
||||
name: "success with filters",
|
||||
state: &gcpCatalogPersistedState{
|
||||
PersistedState: &cred.PersistedState{
|
||||
CredentialsConfig: &cred.Config{
|
||||
ProjectId: "test-project",
|
||||
Zone: "us-central1-a",
|
||||
},
|
||||
},
|
||||
testInstancesAPIFunc: newTestMockInstances(ctx,
|
||||
nil,
|
||||
testMockInstancesWithListInstancesOutput(&computepb.InstanceList{
|
||||
Items: []*computepb.Instance{
|
||||
{
|
||||
Name: pointer("boundary-1"),
|
||||
Id: pointer(uint64(1)),
|
||||
NetworkInterfaces: []*computepb.NetworkInterface{
|
||||
{
|
||||
AccessConfigs: []*computepb.AccessConfig{
|
||||
{
|
||||
NatIP: pointer("102.1.1.1"),
|
||||
ExternalIpv6: pointer("2001:db8::1"),
|
||||
},
|
||||
},
|
||||
Ipv6AccessConfigs: []*computepb.AccessConfig{
|
||||
{
|
||||
ExternalIpv6: pointer("2001:db8::1"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
testMockInstancesWithListInstancesError(nil),
|
||||
),
|
||||
},
|
||||
listFilters: []string{"name = instance-1", "zone = us-central1-a"},
|
||||
},
|
||||
{
|
||||
name: "success without filters",
|
||||
state: &gcpCatalogPersistedState{
|
||||
PersistedState: &cred.PersistedState{
|
||||
CredentialsConfig: &cred.Config{
|
||||
ProjectId: "test-project",
|
||||
Zone: "us-central1-a",
|
||||
},
|
||||
},
|
||||
testInstancesAPIFunc: newTestMockInstances(ctx,
|
||||
nil,
|
||||
testMockInstancesWithListInstancesOutput(&computepb.InstanceList{
|
||||
Items: []*computepb.Instance{
|
||||
{
|
||||
Name: pointer("boundary-1"),
|
||||
Id: pointer(uint64(1)),
|
||||
NetworkInterfaces: []*computepb.NetworkInterface{
|
||||
{
|
||||
AccessConfigs: []*computepb.AccessConfig{
|
||||
{
|
||||
NatIP: pointer("102.1.1.1"),
|
||||
ExternalIpv6: pointer("2001:db8::1"),
|
||||
},
|
||||
},
|
||||
Ipv6AccessConfigs: []*computepb.AccessConfig{
|
||||
{
|
||||
ExternalIpv6: pointer("2001:db8::1"),
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}),
|
||||
testMockInstancesWithListInstancesError(nil),
|
||||
),
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
require := require.New(t)
|
||||
|
||||
st := dryRunValidation(ctx, tc.state, tc.listFilters, tc.clientOptions...)
|
||||
if tc.expectedErr != "" {
|
||||
require.Contains(st.Err().Error(), tc.expectedErr)
|
||||
require.Equal(st.Code(), tc.expectedErrCode)
|
||||
return
|
||||
}
|
||||
require.NoError(st.Err())
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user