From ec90dc83858aed8c6a6a7d0c8b4d9bea936b27e3 Mon Sep 17 00:00:00 2001 From: Alistair Yan <52126234+1riatsila1@users.noreply.github.com> Date: Wed, 2 Oct 2024 11:13:42 +0200 Subject: [PATCH] fix: disallow root group imports (#33) --- .../examples/full-demo/main.tf | 3 -- .../provider/data-sources/group.go | 31 ++++-------- .../provider/resources/group.go | 14 ++---- .../provider/util/group.go | 10 ++++ .../test/data-sources/group_test.go | 50 ++++++------------- .../test/resources/group_test.go | 18 +++++-- 6 files changed, 54 insertions(+), 72 deletions(-) create mode 100644 pkg/config-api-provider/provider/util/group.go diff --git a/pkg/config-api-provider/examples/full-demo/main.tf b/pkg/config-api-provider/examples/full-demo/main.tf index d9b4696f..3a74a0e2 100644 --- a/pkg/config-api-provider/examples/full-demo/main.tf +++ b/pkg/config-api-provider/examples/full-demo/main.tf @@ -13,11 +13,8 @@ provider "uxi" { token_url = "https://test.sso.common.cloud.hpe.com/as/token.oauth2" } -data "uxi_root_group" "my_root_group" {} - resource "uxi_group" "my_group" { name = "parent" - parent_group_id = data.uxi_root_group.my_root_group.id } resource "uxi_group" "my_group_2" { diff --git a/pkg/config-api-provider/provider/data-sources/group.go b/pkg/config-api-provider/provider/data-sources/group.go index 0a83b85a..125aef25 100644 --- a/pkg/config-api-provider/provider/data-sources/group.go +++ b/pkg/config-api-provider/provider/data-sources/group.go @@ -30,7 +30,6 @@ type groupDataSourceModel struct { Name types.String `tfsdk:"name"` Filter struct { GroupID *string `tfsdk:"group_id"` - IsRoot *bool `tfsdk:"is_root"` } `tfsdk:"filter"` } @@ -46,7 +45,6 @@ func (d *groupDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, }, "path": schema.StringAttribute{ Computed: true, - Optional: true, }, "parent_group_id": schema.StringAttribute{ Computed: true, @@ -58,10 +56,7 @@ func (d *groupDataSource) Schema(_ context.Context, _ datasource.SchemaRequest, Required: true, Attributes: map[string]schema.Attribute{ "group_id": schema.StringAttribute{ - Optional: true, - }, - "is_root": schema.BoolAttribute{ - Optional: true, + Required: true, }, }, }, @@ -74,23 +69,14 @@ func (d *groupDataSource) Read(ctx context.Context, req datasource.ReadRequest, // Read configuration from request diags := req.Config.Get(ctx, &state) - if state.Filter.GroupID == nil && (state.Filter.IsRoot == nil || !*state.Filter.IsRoot) { - diags.AddError("invalid Group data source", "either filter.group_id must be set or 'filter.is_root = true' is required") - } else if state.Filter.GroupID != nil && state.Filter.IsRoot != nil && *state.Filter.IsRoot { - diags.AddError("invalid Group data source", "group_id and 'is_root = true' cannot both be set") - } resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return } - request := d.client.ConfigurationAPI.GroupsGetUxiV1alpha1GroupsGet(context.Background()) - - if state.Filter.IsRoot != nil && *state.Filter.IsRoot { - request = request.Uid(*state.Filter.GroupID) // TODO: use root group filter here - } else { - request = request.Uid(*state.Filter.GroupID) - } + request := d.client.ConfigurationAPI. + GroupsGetUxiV1alpha1GroupsGet(context.Background()). + Uid(*state.Filter.GroupID) groupResponse, _, err := util.RetryFor429(request.Execute) @@ -103,12 +89,15 @@ func (d *groupDataSource) Read(ctx context.Context, req datasource.ReadRequest, } group := groupResponse.Items[0] + if util.IsRoot(group) { + resp.Diagnostics.AddError("operation not supported", "the root group cannot be used as a data source") + return + } + state.ID = types.StringValue(group.Id) state.Name = types.StringValue(group.Name) state.Path = types.StringValue(group.Path) - if group.Parent.IsSet() { - state.ParentGroupID = types.StringValue(group.Parent.Get().Id) - } + state.ParentGroupID = types.StringValue(group.Parent.Get().Id) // Set state diags = resp.State.Set(ctx, &state) diff --git a/pkg/config-api-provider/provider/resources/group.go b/pkg/config-api-provider/provider/resources/group.go index 4078044e..ee9a61f7 100644 --- a/pkg/config-api-provider/provider/resources/group.go +++ b/pkg/config-api-provider/provider/resources/group.go @@ -125,9 +125,6 @@ func (r *groupResource) Read(ctx context.Context, req resource.ReadRequest, resp // Get current state var state groupResourceModel diags := req.State.Get(ctx, &state) - if state.ID.ValueString() == GetRootGroupUID() { - diags.AddError("operation not supported", "the root node cannot be used as a resource") - } resp.Diagnostics.Append(diags...) if resp.Diagnostics.HasError() { return @@ -146,9 +143,13 @@ func (r *groupResource) Read(ctx context.Context, req resource.ReadRequest, resp ) return } - group := groupResponse.Items[0] + if util.IsRoot(group) { + resp.Diagnostics.AddError("operation not supported", "the root group cannot be used as a resource") + return + } + // Update state from client response state.Name = types.StringValue(group.Name) state.ParentGroupId = types.StringValue(group.Parent.Get().Id) @@ -215,8 +216,3 @@ var UpdateGroup = func(request GroupUpdateRequestModel) config_api_client.Groups Path: "mock_path", } } - -var GetRootGroupUID = func() string { - // Get root node here using the client - return "root_group_uid" -} diff --git a/pkg/config-api-provider/provider/util/group.go b/pkg/config-api-provider/provider/util/group.go new file mode 100644 index 00000000..94b8387e --- /dev/null +++ b/pkg/config-api-provider/provider/util/group.go @@ -0,0 +1,10 @@ +package util + +import ( + "github.com/aruba-uxi/configuration-api-terraform-provider/pkg/config-api-client" +) + +func IsRoot(group config_api_client.GroupsGetItem) bool { + _, set := group.Parent.Get().GetIdOk() + return !set +} diff --git a/pkg/config-api-provider/test/data-sources/group_test.go b/pkg/config-api-provider/test/data-sources/group_test.go index 11f08b27..6ec3e02b 100644 --- a/pkg/config-api-provider/test/data-sources/group_test.go +++ b/pkg/config-api-provider/test/data-sources/group_test.go @@ -1,6 +1,7 @@ package data_source_test import ( + config_api_client "github.com/aruba-uxi/configuration-api-terraform-provider/pkg/config-api-client" "regexp" "testing" @@ -19,31 +20,7 @@ func TestGroupDataSource(t *testing.T) { resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories, Steps: []resource.TestStep{ - // Test no filters set - { - Config: provider.ProviderConfig + ` - data "uxi_group" "my_group" { - filter = {} - } - `, - ExpectError: regexp.MustCompile(`either filter.group_id must be set or 'filter.is_root = true' is required`), - }, - // Test too many filters set - { - Config: provider.ProviderConfig + ` - data "uxi_group" "my_group" { - filter = { - is_root = true - group_id = "uid" - } - } - `, - ExpectError: regexp.MustCompile(`group_id and 'is_root = true' cannot both be set`), - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr("data.uxi_root_group.my_root_group", "id", "mock_uid"), - ), - }, - // Test Read, is_root not set + // Test Read { PreConfig: func() { util.MockGetGroup( @@ -63,28 +40,29 @@ func TestGroupDataSource(t *testing.T) { resource.TestCheckResourceAttr("data.uxi_group.my_group", "id", "uid"), ), }, - // Test Read, is_root is false + // TODO: Test retrieving the root group { PreConfig: func() { util.MockGetGroup( - "uid", - util.GeneratePaginatedResponse([]map[string]interface{}{util.StructToMap(util.GenerateGroupResponseGetModel("uid", "", ""))}), - 3, + "my_root_group_uid", + util.GeneratePaginatedResponse([]map[string]interface{}{util.StructToMap(config_api_client.GroupsGetItem{ + Id: "my_root_group_uid", + Name: "root", + Parent: *config_api_client.NewNullableParent(nil), + Path: "my_root_group_uid", + })}), + 1, ) }, Config: provider.ProviderConfig + ` data "uxi_group" "my_group" { filter = { - is_root = false - group_id = "uid" + group_id = "my_root_group_uid" } } `, - Check: resource.ComposeAggregateTestCheckFunc( - resource.TestCheckResourceAttr("data.uxi_group.my_group", "id", "uid"), - ), + ExpectError: regexp.MustCompile(`the root group cannot be used as a data source`), }, - // TODO: Test retrieving the root group }, }) @@ -100,7 +78,7 @@ func TestGroupDataSource429Handling(t *testing.T) { ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories, Steps: []resource.TestStep{ - // Test Read, is_root not set + // Test Read { PreConfig: func() { mock429 = gock.New("https://test.api.capenetworks.com"). diff --git a/pkg/config-api-provider/test/resources/group_test.go b/pkg/config-api-provider/test/resources/group_test.go index 235c6fae..6f32adb3 100644 --- a/pkg/config-api-provider/test/resources/group_test.go +++ b/pkg/config-api-provider/test/resources/group_test.go @@ -114,6 +114,8 @@ func TestGroupResource(t *testing.T) { } func TestRootGroupResource(t *testing.T) { + defer gock.Off() + mockOAuth := util.MockOAuth() resource.Test(t, resource.TestCase{ ProtoV6ProviderFactories: provider.TestAccProtoV6ProviderFactories, @@ -121,22 +123,32 @@ func TestRootGroupResource(t *testing.T) { // Importing the root group does not work { PreConfig: func() { - resources.GetRootGroupUID = func() string { return "my_root_group_uid" } + util.MockGetGroup( + "my_root_group_uid", + util.GeneratePaginatedResponse([]map[string]interface{}{util.StructToMap(config_api_client.GroupsGetItem{ + Id: "my_root_group_uid", + Name: "root", + Parent: *config_api_client.NewNullableParent(nil), + Path: "my_root_group_uid", + })}), + 1, + ) }, Config: provider.ProviderConfig + ` resource "uxi_group" "my_root_group" { name = "name" - parent_group_id = "some_random_string" } import { to = uxi_group.my_root_group id = "my_root_group_uid" }`, - ExpectError: regexp.MustCompile(`the root node cannot be used as a resource`), + ExpectError: regexp.MustCompile(`the root group cannot be used as a resource`), }, }, }) + + mockOAuth.Mock.Disable() } func TestGroupResource429Handling(t *testing.T) {