From b03dad0f6e7e9f3e80d6303e1305344762666243 Mon Sep 17 00:00:00 2001 From: James Sammut Date: Thu, 21 Dec 2023 11:55:28 +1100 Subject: [PATCH 1/3] Plan to state for organization allowed IP create/updates --- buildkite/resource_organization.go | 16 ++-------------- go.mod | 7 +++++++ go.sum | 15 +++++++++++++++ schema.graphql | 27 ++++++++++++++------------- 4 files changed, 38 insertions(+), 27 deletions(-) diff --git a/buildkite/resource_organization.go b/buildkite/resource_organization.go index 0430078e..a7e2d1bf 100644 --- a/buildkite/resource_organization.go +++ b/buildkite/resource_organization.go @@ -128,13 +128,7 @@ func (o *organizationResource) Create(ctx context.Context, req resource.CreateRe state.ID = types.StringValue(apiResponse.OrganizationApiIpAllowlistUpdate.Organization.Id) state.UUID = types.StringValue(apiResponse.OrganizationApiIpAllowlistUpdate.Organization.Uuid) state.Enforce2FA = plan.Enforce2FA - ips, diag := types.ListValueFrom(ctx, types.StringType, strings.Split(apiResponse.OrganizationApiIpAllowlistUpdate.Organization.AllowedApiIpAddresses, " ")) - state.AllowedApiIpAddresses = ips - - if diag.HasError() { - resp.Diagnostics.Append(diag...) - return - } + state.AllowedApiIpAddresses = plan.AllowedApiIpAddresses resp.Diagnostics.Append(resp.State.Set(ctx, state)...) } @@ -240,13 +234,7 @@ func (o *organizationResource) Update(ctx context.Context, req resource.UpdateRe state.ID = types.StringValue(apiResponse.OrganizationApiIpAllowlistUpdate.Organization.Id) state.UUID = types.StringValue(apiResponse.OrganizationApiIpAllowlistUpdate.Organization.Uuid) - ips, diag := types.ListValueFrom(ctx, types.StringType, strings.Split(apiResponse.OrganizationApiIpAllowlistUpdate.Organization.AllowedApiIpAddresses, " ")) - state.AllowedApiIpAddresses = ips - - if diag.HasError() { - resp.Diagnostics.Append(diag...) - return - } + state.AllowedApiIpAddresses = plan.AllowedApiIpAddresses resp.Diagnostics.Append(resp.State.Set(ctx, state)...) } diff --git a/go.mod b/go.mod index fe97c6d0..dd391d65 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,9 @@ require ( require ( github.com/Microsoft/go-winio v0.6.1 // indirect + github.com/agnivade/levenshtein v1.1.1 // indirect + github.com/alexflint/go-arg v1.4.2 // indirect + github.com/alexflint/go-scalar v1.0.0 // indirect github.com/buildkite/interpolate v0.0.0-20200526001904-07f35b4ae251 // indirect github.com/decred/dcrd/dcrec/secp256k1/v4 v4.2.0 // indirect github.com/goccy/go-json v0.10.2 // indirect @@ -31,8 +34,12 @@ require ( github.com/lestrrat-go/option v1.0.1 // indirect github.com/oleiade/reflections v1.0.1 // indirect github.com/segmentio/asm v1.2.0 // indirect + github.com/suessflorian/gqlfetch v0.6.0 // indirect + github.com/vektah/gqlparser v1.3.1 // indirect golang.org/x/exp v0.0.0-20231108232855-2478ac86f678 // indirect + golang.org/x/tools v0.15.0 // indirect google.golang.org/genproto/googleapis/rpc v0.0.0-20231009173412-8bfb1ae86b6c // indirect + gopkg.in/yaml.v2 v2.4.0 // indirect ) require ( diff --git a/go.sum b/go.sum index 3a155ca5..1514b2bb 100644 --- a/go.sum +++ b/go.sum @@ -10,7 +10,13 @@ github.com/acomagu/bufpipe v1.0.4 h1:e3H4WUzM3npvo5uv95QuJM3cQspFNtFBzvJ2oNjKIDQ github.com/acomagu/bufpipe v1.0.4/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ2sYmHc4= github.com/agext/levenshtein v1.2.2 h1:0S/Yg6LYmFJ5stwQeRp6EeOcCbj7xiqQSdNelsXvaqE= github.com/agext/levenshtein v1.2.2/go.mod h1:JEDfjyjHDjOF/1e4FlBE/PkbqA9OfWu2ki2W0IB5558= +github.com/agnivade/levenshtein v1.0.1/go.mod h1:CURSv5d9Uaml+FovSIICkLbAUZ9S4RqaHDIsdSBg7lM= +github.com/agnivade/levenshtein v1.1.1 h1:QY8M92nrzkmr798gCo3kmMyqXFzdQVpxLlGPRBij0P8= github.com/agnivade/levenshtein v1.1.1/go.mod h1:veldBMzWxcCG2ZvUTKD2kJNRdCk5hVbJomOvKkmgYbo= +github.com/alexflint/go-arg v1.4.2 h1:lDWZAXxpAnZUq4qwb86p/3rIJJ2Li81EoMbTMujhVa0= +github.com/alexflint/go-arg v1.4.2/go.mod h1:9iRbDxne7LcR/GSvEr7ma++GLpdIU1zrghf2y2768kM= +github.com/alexflint/go-scalar v1.0.0 h1:NGupf1XV/Xb04wXskDFzS0KWOLH632W/EO4fAFi+A70= +github.com/alexflint/go-scalar v1.0.0/go.mod h1:GpHzbCOZXEKMEcygYQ5n/aa4Aq84zbxjy3MxYW0gjYw= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883 h1:bvNMNQO63//z+xNgfBlViaCIJKLlCJ6/fmUseuG0wVQ= github.com/andreyvit/diff v0.0.0-20170406064948-c7f18ee00883/go.mod h1:rCTlJbsFo29Kk6CurOXKm700vrz8f0KW0JNfpkRJY/8= github.com/apparentlymart/go-textseg/v12 v12.0.0/go.mod h1:S/4uRK2UtaQttw1GenVJEynmyUenKwP++x/+DdGV/Ec= @@ -173,6 +179,7 @@ github.com/rogpeppe/go-internal v1.6.1 h1:/FiVV8dS/e+YqF2JvO3yXRFbBLTIuSDkuC7aBO github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= github.com/segmentio/asm v1.2.0 h1:9BQrFxC+YOHJlTlHGkTrFWf59nbL3XnCoFLTwDCI7ys= github.com/segmentio/asm v1.2.0/go.mod h1:BqMnlJP91P8d+4ibuonYZw9mfnzI9HfxselHZr5aAcs= +github.com/sergi/go-diff v1.0.0/go.mod h1:0CfEIISq7TuYL3j771MWULgwwjU+GofnZX9QAmXWZgo= github.com/sergi/go-diff v1.3.1 h1:xkr+Oxo4BOQKmkn/B9eMK0g5Kg/983T9DqqPHwYqD+8= github.com/sergi/go-diff v1.3.1/go.mod h1:aMJSSKb2lpPvRNec0+w3fl7LP9IOFzdc9Pa4NFbPK1I= github.com/shurcooL/graphql v0.0.0-20181231061246-d48a9a75455f h1:tygelZueB1EtXkPI6mQ4o9DQ0+FKW41hTbunoXZCTqk= @@ -182,6 +189,8 @@ github.com/skeema/knownhosts v1.1.0/go.mod h1:sKFq3RD6/TKZkSWn8boUbDC7Qkgcv+8XXi github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= +github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs= +github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI= github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4= github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= @@ -190,6 +199,10 @@ github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1F github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= +github.com/suessflorian/gqlfetch v0.6.0 h1:6e+Oe9mWbbjSmJez+6I4tyskQMy6lQlFFQYj64gaCQU= +github.com/suessflorian/gqlfetch v0.6.0/go.mod h1:Xlz+o2ate8M/Hr237HJpFyJD0l05uh3NAX3zmXVmjxU= +github.com/vektah/gqlparser v1.3.1 h1:8b0IcD3qZKWJQHSzynbDlrtP3IxVydZ2DZepCGofqfU= +github.com/vektah/gqlparser v1.3.1/go.mod h1:bkVf0FX+Stjg/MHnm8mEyubuaArhNEqfQhF+OTiAL74= github.com/vektah/gqlparser/v2 v2.5.8 h1:pm6WOnGdzFOCfcQo9L3+xzW51mKrlwTEg4Wr7AH1JW4= github.com/vektah/gqlparser/v2 v2.5.8/go.mod h1:z8xXUff237NntSuH8mLFijZ+1tjV1swDbpDqjJmk6ME= github.com/vmihailenco/msgpack v3.3.3+incompatible/go.mod h1:fy3FlTQTDXWkZ7Bh6AcGMlsjHatGryHQYUTf1ShIgkk= @@ -261,6 +274,7 @@ golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20190125232054-d66bd3c5d5a6/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= @@ -288,6 +302,7 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/warnings.v0 v0.1.2 h1:wFXVbFY8DY5/xOe1ECiWdKCzZlxgshcYVNkBHstARME= gopkg.in/warnings.v0 v0.1.2/go.mod h1:jksf8JmL6Qr/oQM2OXTHunEvvTAsrWBLb6OOjuVWRNI= gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= +gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY= gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= diff --git a/schema.graphql b/schema.graphql index e1291254..75367bb0 100644 --- a/schema.graphql +++ b/schema.graphql @@ -380,6 +380,8 @@ type ArtifactEdge { """Context for an audit event created during an REST/GraphQL API request""" type AuditAPIContext { +"""The API access token UUID used to authenticate the request""" + requestApiAccessTokenUuid: String """The remote IP which made the request""" requestIpAddress: String """The client supplied user agent which made the request""" @@ -556,9 +558,11 @@ type AuditSubject { union AuditSubjectNode =APIAccessToken | AgentToken | AuthorizationBitbucket | AuthorizationGitHub | AuthorizationGitHubEnterprise | Cluster | ClusterPermission | ClusterQueue | ClusterQueueToken | ClusterToken | Email | NotificationServiceSlack | NotificationServiceWebhook | Organization | OrganizationBanner | OrganizationInvitation | OrganizationMember | Pipeline | PipelineSchedule | PipelineTemplate | SCMPipelineSettings | SCMRepositoryHost | SCMService | SSOProviderGitHubApp | SSOProviderGoogleGSuite | SSOProviderSAML | Subscription | Suite | TOTP | Team | TeamMember | TeamPipeline | TeamSuite | User """All the possible types of subjects in an Audit Event""" -enum AuditSubjectType { - TEAM - ORGANIZATION_MEMBER +enum AuditSubjectType { + ORGANIZATION_MEMBER + SSO_PROVIDER + CLUSTER_PERMISSION + PIPELINE AGENT_TOKEN API_ACCESS_TOKEN CLUSTER_QUEUE @@ -567,27 +571,24 @@ enum AuditSubjectType { NOTIFICATION_SERVICE ORGANIZATION_BANNER ORGANIZATION_INVITATION - PIPELINE_SCHEDULE - CLUSTER_PERMISSION + PIPELINE_SCHEDULE PIPELINE_TEMPLATE TEAM_MEMBER TEAM_PIPELINE - SCM_SERVICE - PIPELINE TEAM_SUITE - SCM_REPOSITORY_HOST + SCM_SERVICE SCM_PIPELINE_SETTINGS + ORGANIZATION + SCM_REPOSITORY_HOST + SUBSCRIPTION USER_EMAIL SUITE_MONITOR - USER_TOTP - SUBSCRIPTION - ORGANIZATION SUITE + USER_TOTP USER TEAM - ORGANIZATION_MEMBER AUTHORIZATION - CLUSTER + CLUSTER } """Context for an audit event created during a web request""" From f222df56715bfd344852354c69f781e35ca296d7 Mon Sep 17 00:00:00 2001 From: James Sammut Date: Thu, 21 Dec 2023 12:48:01 +1100 Subject: [PATCH 2/3] Moved organization tests to t.Run(), empty string list/unset allowed API IP tests added --- CHANGELOG.md | 1 + buildkite/resource_organization_test.go | 251 +++++++++++++++++------- 2 files changed, 179 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 50cc7d11..b1db4f30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased - SUP-1657: Removal of non-admin tests [[PR #457](https://github.com/buildkite/terraform-provider-buildkite/pull/457)] @james2791 +- SUP-1628: Adjustment of Organization resource Allowed API IP address state persistence [[PR #458](https://github.com/buildkite/terraform-provider-buildkite/pull/458)] @james2791 ## [v1.2.0](https://github.com/buildkite/terraform-provider-buildkite/compare/v1.1.1...v1.2.0) diff --git a/buildkite/resource_organization_test.go b/buildkite/resource_organization_test.go index 1f656541..1c99d1e7 100644 --- a/buildkite/resource_organization_test.go +++ b/buildkite/resource_organization_test.go @@ -11,90 +11,195 @@ import ( "github.com/hashicorp/terraform-plugin-testing/terraform" ) -func TestAccOrganization_create(t *testing.T) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: protoV6ProviderFactories(), - CheckDestroy: testCheckOrganizationResourceRemoved, - Steps: []resource.TestStep{ - { - Config: testAccOrganizationConfigBasic([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), - Check: resource.ComposeAggregateTestCheckFunc( - // Confirm that the allowed IP addresses are set correctly in Buildkite's system - testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), - // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 - // allowing us to assert the first IP is also added correctly - resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.1", "1.1.1.1/32"), - ), +func TestAccBuildkiteOrganizationResource(t *testing.T) { + + config := func(ip_addresses []string) string { + config := ` + + provider "buildkite" { + timeouts = { + create = "10s" + read = "10s" + update = "10s" + delete = "10s" + } + } + + resource "buildkite_organization" "let_them_in" { + allowed_api_ip_addresses = %v + } + ` + marshal, _ := json.Marshal(ip_addresses) + + return fmt.Sprintf(config, string(marshal)) + } + + configNoAllowedIPs := func() string { + config := ` + + provider "buildkite" { + timeouts = { + create = "10s" + read = "10s" + update = "10s" + delete = "10s" + } + } + + resource "buildkite_organization" "let_them_in" {} + ` + + return fmt.Sprintf(config) + } + + t.Run("creates an organization", func(t *testing.T) { + check := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 + // allowing us to assert the first IP is also added correctly + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.1", "1.1.1.1/32"), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: protoV6ProviderFactories(), + CheckDestroy: testCheckOrganizationResourceRemoved, + Steps: []resource.TestStep{ + { + Config: config([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + Check: check, + }, }, - }, + }) }) -} -func TestAccOrganization_update(t *testing.T) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: protoV6ProviderFactories(), - CheckDestroy: testCheckOrganizationResourceRemoved, - Steps: []resource.TestStep{ - { - Config: testAccOrganizationConfigBasic([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), - Check: resource.ComposeAggregateTestCheckFunc( - // Confirm that the allowed IP addresses are set correctly in Buildkite's system - testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), - // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 - // allowing us to assert the first IP is also added correctly - resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.2", "1.0.0.1/32"), - ), - }, - { - Config: testAccOrganizationConfigBasic([]string{"0.0.0.0/0", "4.4.4.4/32"}), - Check: resource.ComposeAggregateTestCheckFunc( - // Confirm that the allowed IP addresses are set correctly in Buildkite's system - testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "4.4.4.4/32"}), - // This check allows us to ensure that TF still has access (0.0.0.0/0) and that the new IP address is added correctly - resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.1", "4.4.4.4/32"), - ), + t.Run("updates an organization", func(t *testing.T) { + check := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 + // allowing us to assert the first IP is also added correctly + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.2", "1.0.0.1/32"), + ) + + ckeckUpdated := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "4.4.4.4/32"}), + // This check allows us to ensure that TF still has access (0.0.0.0/0) and that the new IP address is added correctly + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.1", "4.4.4.4/32"), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: protoV6ProviderFactories(), + CheckDestroy: testCheckOrganizationResourceRemoved, + Steps: []resource.TestStep{ + { + Config: config([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + Check: check, + }, + { + Config: config([]string{"0.0.0.0/0", "4.4.4.4/32"}), + Check: ckeckUpdated, + }, }, - }, + }) }) -} -func TestAccOrganization_import(t *testing.T) { - resource.Test(t, resource.TestCase{ - PreCheck: func() { testAccPreCheck(t) }, - ProtoV6ProviderFactories: protoV6ProviderFactories(), - CheckDestroy: testCheckOrganizationResourceRemoved, - Steps: []resource.TestStep{ - { - Config: testAccOrganizationConfigBasic([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), - Check: resource.ComposeAggregateTestCheckFunc( - // Confirm that the allowed IP addresses are set correctly in Buildkite's system - testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), - // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 - // allowing us to assert the first IP is also added correctly - resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.2", "1.0.0.1/32"), - ), - }, - { - ResourceName: "buildkite_organization.let_them_in", - ImportState: true, - ImportStateVerify: true, + t.Run("updates an organization with unset allowed API IP address list", func(t *testing.T) { + check := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 + // allowing us to assert the first IP is also added correctly + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.2", "1.0.0.1/32"), + ) + + ckeckUpdated := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{""}), + // Check the allowed IP address list in state is of length 1, and a empty string element + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.#", "1"), + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.0", ""), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: protoV6ProviderFactories(), + CheckDestroy: testCheckOrganizationResourceRemoved, + Steps: []resource.TestStep{ + { + Config: config([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + Check: check, + }, + { + Config: config([]string{""}), + Check: ckeckUpdated, + }, }, - }, + }) }) -} -func testAccOrganizationConfigBasic(ip_addresses []string) string { - config := ` + t.Run("updates an organization with cleared allowed API IP address list", func(t *testing.T) { + check := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 + // allowing us to assert the first IP is also added correctly + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.2", "1.0.0.1/32"), + ) - resource "buildkite_organization" "let_them_in" { - allowed_api_ip_addresses = %v - } - ` - marshal, _ := json.Marshal(ip_addresses) + ckeckUpdated := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{""}), + // Check the allowed IP address list in not set in state + resource.TestCheckNoResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses"), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: protoV6ProviderFactories(), + CheckDestroy: testCheckOrganizationResourceRemoved, + Steps: []resource.TestStep{ + { + Config: config([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + Check: check, + }, + { + Config: configNoAllowedIPs(), + Check: ckeckUpdated, + }, + }, + }) + }) - return fmt.Sprintf(config, string(marshal)) + t.Run("imports an organization", func(t *testing.T) { + check := resource.ComposeAggregateTestCheckFunc( + // Confirm that the allowed IP addresses are set correctly in Buildkite's system + testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + // Check that the second IP added to the list is the one we expect, 0.0.0.0/0, this also ensures the length is greater than 1 + // allowing us to assert the first IP is also added correctly + resource.TestCheckResourceAttr("buildkite_organization.let_them_in", "allowed_api_ip_addresses.2", "1.0.0.1/32"), + ) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: protoV6ProviderFactories(), + CheckDestroy: testCheckOrganizationResourceRemoved, + Steps: []resource.TestStep{ + { + Config: config([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), + Check: check, + }, + { + ResourceName: "buildkite_organization.let_them_in", + ImportState: true, + ImportStateVerify: true, + }, + }, + }) + }) } func testCheckOrganizationResourceRemoved(s *terraform.State) error { From 0a715fa226966f4a140d02a3c97542eac9f1386b Mon Sep 17 00:00:00 2001 From: James Sammut Date: Thu, 21 Dec 2023 14:51:26 +1100 Subject: [PATCH 3/3] Test name tweak, logic adjust for clearing API allowlist property test --- buildkite/resource_organization_test.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/buildkite/resource_organization_test.go b/buildkite/resource_organization_test.go index 1c99d1e7..83c62567 100644 --- a/buildkite/resource_organization_test.go +++ b/buildkite/resource_organization_test.go @@ -107,7 +107,7 @@ func TestAccBuildkiteOrganizationResource(t *testing.T) { }) }) - t.Run("updates an organization with unset allowed API IP address list", func(t *testing.T) { + t.Run("updates an organization with an empty string allowed API IP address list", func(t *testing.T) { check := resource.ComposeAggregateTestCheckFunc( // Confirm that the allowed IP addresses are set correctly in Buildkite's system testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), @@ -141,7 +141,7 @@ func TestAccBuildkiteOrganizationResource(t *testing.T) { }) }) - t.Run("updates an organization with cleared allowed API IP address list", func(t *testing.T) { + t.Run("updates an organization by removing the allowed API IP address list property", func(t *testing.T) { check := resource.ComposeAggregateTestCheckFunc( // Confirm that the allowed IP addresses are set correctly in Buildkite's system testAccCheckOrganizationRemoteValues([]string{"0.0.0.0/0", "1.1.1.1/32", "1.0.0.1/32"}), @@ -169,6 +169,8 @@ func TestAccBuildkiteOrganizationResource(t *testing.T) { { Config: configNoAllowedIPs(), Check: ckeckUpdated, + // After clearing the IPs, state will be set to null and refresh will restore the attribute to an empty-string list of length 1 + ExpectNonEmptyPlan: true, }, }, })