From 99c753ac42cc28c7ec9c16dd9067a901ba0d84de Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Fri, 5 Apr 2024 09:52:06 -0700 Subject: [PATCH] Add validator for IP/CIDR --- pkg/platform/resource_myjfrog_ip_allowlist.go | 263 ++++++++++-------- .../resource_myjfrog_ip_allowlist_test.go | 123 ++++++-- 2 files changed, 244 insertions(+), 142 deletions(-) diff --git a/pkg/platform/resource_myjfrog_ip_allowlist.go b/pkg/platform/resource_myjfrog_ip_allowlist.go index 963a89e..da71ff6 100644 --- a/pkg/platform/resource_myjfrog_ip_allowlist.go +++ b/pkg/platform/resource_myjfrog_ip_allowlist.go @@ -4,12 +4,14 @@ import ( "context" "encoding/json" "fmt" + "net" "net/http" "slices" "strings" "time" "github.com/go-resty/resty/v2" + "github.com/hashicorp/terraform-plugin-framework-validators/setvalidator" "github.com/hashicorp/terraform-plugin-framework-validators/stringvalidator" "github.com/hashicorp/terraform-plugin-framework/diag" "github.com/hashicorp/terraform-plugin-framework/path" @@ -101,6 +103,11 @@ func (r *ipAllowListResource) Schema(ctx context.Context, req resource.SchemaReq ElementType: types.StringType, Required: true, Description: "List of IPs for the JPD allowlist", + Validators: []validator.Set{ + setvalidator.ValueStringsAre( + IPCIDR(), + ), + }, }, }, MarkdownDescription: "Provides a MyJFrog [IP allowlist](https://jfrog.com/help/r/jfrog-hosting-models-documentation/configure-the-ip/cidr-allowlist) resource to manage list of allow IP/CIDR addresses. " + @@ -161,7 +168,7 @@ func (r ipAllowListResource) waitForCompletion(ctx context.Context, serverName s var retryFunc = func(_ context.Context) error { tflog.Info(ctx, "checking ip allowlist process status") - i, status, err := r.getAllowList(serverName) + i, status, err := r.getIPs(serverName) if err != nil { return err } @@ -182,88 +189,111 @@ func (r ipAllowListResource) waitForCompletion(ctx context.Context, serverName s return ips, nil } -func (r *ipAllowListResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { - var plan ipAllowListResourceModel +func (r *ipAllowListResource) getIPs(serverName string) ([]string, string, error) { + var result ipAllowListAPIGetResponseModel - resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) - if resp.Diagnostics.HasError() { - return + response, err := r.ProviderData.MyJFrogClient.R(). + SetPathParam("serverName", serverName). + SetResult(&result). + Get(ipAllowlistEndpoint) + + if err != nil { + return nil, "", err } - var planIPs []string - resp.Diagnostics.Append(plan.IPs.ElementsAs(ctx, &planIPs, false)...) - if resp.Diagnostics.HasError() { - return + if response.IsError() { + return nil, "", fmt.Errorf("%s", response.String()) + } + + ips := lo.Map(result.IPs, func(list ipAllowListIPAPIGetResponseModel, _ int) string { + return list.IP + }) + return ips, result.Status, nil +} + +func (r *ipAllowListResource) addIPs(ctx context.Context, serverName string, ips []string) ([]string, error) { + return r.mutateIPs(ctx, serverName, ips, func(req *resty.Request) (*resty.Response, error) { + return req.Post(ipAllowlistEndpoint) + }) +} + +func (r *ipAllowListResource) removeIPs(ctx context.Context, serverName string, ips []string) ([]string, error) { + return r.mutateIPs(ctx, serverName, ips, func(req *resty.Request) (*resty.Response, error) { + return req.Delete(ipAllowlistEndpoint) + }) +} + +func (r *ipAllowListResource) mutateIPs(ctx context.Context, serverName string, ips []string, requestCallback func(req *resty.Request) (*resty.Response, error)) ([]string, error) { + if requestCallback == nil { + return nil, fmt.Errorf("requestCallback cannot be nil") } allowList := ipAllowListAPIPostRequestModel{ - IPs: planIPs, + IPs: ips, } - serverName := plan.ServerName.ValueString() var result ipAllowListAPIPostDeleteResponseModel var apiErr MyJFrogAllowlistErrorResponse - response, err := r.ProviderData.MyJFrogClient.R(). + req := r.ProviderData.MyJFrogClient.R(). SetPathParam("serverName", serverName). SetBody(&allowList). SetResult(&result). - SetError(&apiErr). - Post(ipAllowlistEndpoint) + SetError(&apiErr) + + resp, err := requestCallback(req) if err != nil { - utilfw.UnableToCreateResourceError(resp, err.Error()) - return + return nil, err } - if response.IsError() { - if slices.Contains([]int{http.StatusConflict, http.StatusTooManyRequests}, response.StatusCode()) { + if resp.IsError() { + if slices.Contains([]int{http.StatusConflict, http.StatusTooManyRequests}, resp.StatusCode()) { var conflictErr MyJFrogAllowlistConflictErrorResponse - err := json.Unmarshal(response.Body(), &conflictErr) + err := json.Unmarshal(resp.Body(), &conflictErr) if err != nil { - utilfw.UnableToCreateResourceError(resp, err.Error()) - return + return nil, err } - utilfw.UnableToCreateResourceError(resp, conflictErr.Error()) - return + return nil, conflictErr } - utilfw.UnableToCreateResourceError(resp, apiErr.Error()) - return + return nil, apiErr } - ips, err := r.waitForCompletion(ctx, serverName) + updatedIPS, err := r.waitForCompletion(ctx, serverName) if err != nil { - utilfw.UnableToCreateResourceError(resp, err.Error()) - return - } - - resp.Diagnostics.Append(plan.fromIPs(ctx, ips)...) - if resp.Diagnostics.HasError() { - return + return nil, err } - resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) + return updatedIPS, nil } -func (r *ipAllowListResource) getAllowList(serverName string) ([]string, string, error) { - var result ipAllowListAPIGetResponseModel +func (r *ipAllowListResource) Create(ctx context.Context, req resource.CreateRequest, resp *resource.CreateResponse) { + var plan ipAllowListResourceModel - response, err := r.ProviderData.MyJFrogClient.R(). - SetPathParam("serverName", serverName). - SetResult(&result). - Get(ipAllowlistEndpoint) + resp.Diagnostics.Append(req.Plan.Get(ctx, &plan)...) + if resp.Diagnostics.HasError() { + return + } - if err != nil { - return nil, "", err + var planIPs []string + resp.Diagnostics.Append(plan.IPs.ElementsAs(ctx, &planIPs, false)...) + if resp.Diagnostics.HasError() { + return } - if response.IsError() { - return nil, "", fmt.Errorf("%s", response.String()) + if len(planIPs) > 0 { + ips, err := r.addIPs(ctx, plan.ServerName.ValueString(), planIPs) + if err != nil { + utilfw.UnableToCreateResourceError(resp, err.Error()) + return + } + + resp.Diagnostics.Append(plan.fromIPs(ctx, ips)...) + if resp.Diagnostics.HasError() { + return + } } - ips := lo.Map(result.IPs, func(list ipAllowListIPAPIGetResponseModel, _ int) string { - return list.IP - }) - return ips, result.Status, nil + resp.Diagnostics.Append(resp.State.Set(ctx, &plan)...) } func (r *ipAllowListResource) Read(ctx context.Context, req resource.ReadRequest, resp *resource.ReadResponse) { @@ -302,7 +332,7 @@ func (r *ipAllowListResource) Update(ctx context.Context, req resource.UpdateReq serverName := plan.ServerName.ValueString() // Get the current list of IPs - jpdIPs, _, err := r.getAllowList(serverName) + jpdIPs, _, err := r.getIPs(serverName) if err != nil { utilfw.UnableToUpdateResourceError(resp, err.Error()) return @@ -320,56 +350,31 @@ func (r *ipAllowListResource) Update(ctx context.Context, req resource.UpdateReq } ipsToAdd, ipsToRemove := lo.Difference(planIPs, jpdIPs) - var result ipAllowListAPIPostDeleteResponseModel - var response *resty.Response - var apiErr MyJFrogAllowlistErrorResponse - - myJFrogReq := r.ProviderData.MyJFrogClient.R(). - SetPathParam("serverName", plan.ServerName.ValueString()). - SetResult(&result). - SetError(&apiErr) if len(ipsToAdd) > 0 { - allowList := ipAllowListAPIPostRequestModel{ - IPs: ipsToAdd, + _, e := r.addIPs(ctx, serverName, ipsToAdd) + if e != nil { + resp.Diagnostics.AddError( + "failed to add IPs", + e.Error(), + ) } - - response, err = myJFrogReq. - SetBody(&allowList). - Post(ipAllowlistEndpoint) } - if len(ipsToRemove) > 0 { - allowList := ipAllowListAPIPostRequestModel{ - IPs: ipsToRemove, + _, e := r.removeIPs(ctx, serverName, ipsToRemove) + if e != nil { + resp.Diagnostics.AddError( + "failed to add IPs", + e.Error(), + ) } - - response, err = myJFrogReq. - SetBody(&allowList). - Delete(ipAllowlistEndpoint) } - if err != nil { - utilfw.UnableToUpdateResourceError(resp, err.Error()) - return - } - - if response.IsError() { - if slices.Contains([]int{http.StatusConflict, http.StatusTooManyRequests}, response.StatusCode()) { - var conflictErr MyJFrogAllowlistConflictErrorResponse - err := json.Unmarshal(response.Body(), &conflictErr) - if err != nil { - utilfw.UnableToUpdateResourceError(resp, err.Error()) - return - } - utilfw.UnableToUpdateResourceError(resp, conflictErr.Error()) - return - } - - utilfw.UnableToUpdateResourceError(resp, apiErr.Error()) + if resp.Diagnostics.HasError() { return } + // waitForCompletion fetches list of ips from server so no need for extra GET request ips, err := r.waitForCompletion(ctx, serverName) if err != nil { utilfw.UnableToUpdateResourceError(resp, err.Error()) @@ -399,50 +404,64 @@ func (r *ipAllowListResource) Delete(ctx context.Context, req resource.DeleteReq return } - allowList := ipAllowListAPIPostRequestModel{ - IPs: ipsToRemove, + if len(ipsToRemove) > 0 { + _, err := r.removeIPs(ctx, state.ServerName.ValueString(), ipsToRemove) + if err != nil { + utilfw.UnableToDeleteResourceError(resp, err.Error()) + } } - serverName := state.ServerName.ValueString() - var result ipAllowListAPIPostDeleteResponseModel - var apiErr MyJFrogAllowlistErrorResponse + // If the logic reaches here, it implicitly succeeded and will remove + // the resource from state if there are no other errors. +} - response, err := r.ProviderData.MyJFrogClient.R(). - SetPathParam("serverName", serverName). - SetBody(&allowList). - SetResult(&result). - SetError(&apiErr). - Delete(ipAllowlistEndpoint) - if err != nil { - utilfw.UnableToDeleteResourceError(resp, err.Error()) +func (r *ipAllowListResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { + resource.ImportStatePassthroughID(ctx, path.Root("server_name"), req, resp) +} + +type ipCIDRValidator struct{} + +// Description returns a plain text description of the validator's behavior, suitable for a practitioner to understand its impact. +func (v ipCIDRValidator) Description(ctx context.Context) string { + return `IP/CIDR must be in format like "192.0.2.0/24" or "2001:db8::/32", as defined in RFC 4632 and RFC 4291.` +} + +// MarkdownDescription returns a markdown formatted description of the validator's behavior, suitable for a practitioner to understand its impact. +func (v ipCIDRValidator) MarkdownDescription(ctx context.Context) string { + return `IP/CIDR must be in format like "192.0.2.0/24" or "2001:db8::/32", as defined in RFC 4632 and RFC 4291.` +} + +func (v ipCIDRValidator) ValidateString(ctx context.Context, req validator.StringRequest, resp *validator.StringResponse) { + // If the value is unknown or null, there is nothing to validate. + if req.ConfigValue.IsUnknown() || req.ConfigValue.IsNull() { return } - if response.IsError() { - if slices.Contains([]int{http.StatusConflict, http.StatusTooManyRequests}, response.StatusCode()) { - var conflictErr MyJFrogAllowlistConflictErrorResponse - err := json.Unmarshal(response.Body(), &conflictErr) - if err != nil { - utilfw.UnableToDeleteResourceError(resp, err.Error()) - return - } - utilfw.UnableToDeleteResourceError(resp, conflictErr.Error()) - return - } + isValidIP := true + isValidCIDR := true + var err error - utilfw.UnableToDeleteResourceError(resp, apiErr.Error()) - return + ip := net.ParseIP(req.ConfigValue.ValueString()) + if ip == nil { + isValidIP = false + err = fmt.Errorf("invalid IP address: %s", req.ConfigValue.ValueString()) } - if _, err := r.waitForCompletion(ctx, serverName); err != nil { - utilfw.UnableToDeleteResourceError(resp, err.Error()) - return + _, _, e := net.ParseCIDR(req.ConfigValue.ValueString()) + if e != nil { + isValidCIDR = false + err = e } - // If the logic reaches here, it implicitly succeeded and will remove - // the resource from state if there are no other errors. + if !isValidIP && !isValidCIDR { + resp.Diagnostics.AddAttributeError( + req.Path, + "Invalid IP/CIDR format", + err.Error(), + ) + } } -func (r *ipAllowListResource) ImportState(ctx context.Context, req resource.ImportStateRequest, resp *resource.ImportStateResponse) { - resource.ImportStatePassthroughID(ctx, path.Root("server_name"), req, resp) +func IPCIDR() ipCIDRValidator { + return ipCIDRValidator{} } diff --git a/pkg/platform/resource_myjfrog_ip_allowlist_test.go b/pkg/platform/resource_myjfrog_ip_allowlist_test.go index b6a1f6d..3c2b657 100644 --- a/pkg/platform/resource_myjfrog_ip_allowlist_test.go +++ b/pkg/platform/resource_myjfrog_ip_allowlist_test.go @@ -38,7 +38,7 @@ func TestAccIPAllowlist_full(t *testing.T) { testData := map[string]string{ "name": allowlistName, "serverName": serverName, - "ips": `["1.1.1.7/1"]`, + "ips": `["1.1.1.7", "2.2.2.7/1"]`, } config := testutil.ExecuteTemplate(allowlistName, temp, testData) @@ -46,17 +46,10 @@ func TestAccIPAllowlist_full(t *testing.T) { updatedTestData := map[string]string{ "name": allowlistName, "serverName": serverName, - "ips": `["1.1.1.7/1", "2.2.2.7/1"]`, + "ips": `["2.2.2.7/1", "3.3.3.7/1"]`, } updatedConfig := testutil.ExecuteTemplate(allowlistName, temp, updatedTestData) - updatedTestData2 := map[string]string{ - "name": allowlistName, - "serverName": serverName, - "ips": `["2.2.2.7/1"]`, - } - updatedConfig2 := testutil.ExecuteTemplate(allowlistName, temp, updatedTestData2) - resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ProtoV6ProviderFactories: testAccProviders(), @@ -65,8 +58,9 @@ func TestAccIPAllowlist_full(t *testing.T) { Config: config, Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(fqrn, "server_name", testData["serverName"]), - resource.TestCheckResourceAttr(fqrn, "ips.#", "1"), - resource.TestCheckResourceAttr(fqrn, "ips.0", "1.1.1.7/1"), + resource.TestCheckResourceAttr(fqrn, "ips.#", "2"), + resource.TestCheckTypeSetElemAttr(fqrn, "ips.*", "1.1.1.7"), + resource.TestCheckTypeSetElemAttr(fqrn, "ips.*", "2.2.2.7/1"), ), }, { @@ -74,16 +68,8 @@ func TestAccIPAllowlist_full(t *testing.T) { Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttr(fqrn, "server_name", updatedTestData["serverName"]), resource.TestCheckResourceAttr(fqrn, "ips.#", "2"), - resource.TestCheckTypeSetElemAttr(fqrn, "ips.*", "1.1.1.7/1"), resource.TestCheckTypeSetElemAttr(fqrn, "ips.*", "2.2.2.7/1"), - ), - }, - { - Config: updatedConfig2, - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr(fqrn, "server_name", testData["serverName"]), - resource.TestCheckResourceAttr(fqrn, "ips.#", "1"), - resource.TestCheckResourceAttr(fqrn, "ips.0", "2.2.2.7/1"), + resource.TestCheckTypeSetElemAttr(fqrn, "ips.*", "3.3.3.7/1"), ), }, { @@ -96,3 +82,100 @@ func TestAccIPAllowlist_full(t *testing.T) { }, }) } + +func TestAccIPAllowlist_empty_ips(t *testing.T) { + jfrogURL := os.Getenv("JFROG_URL") + if !strings.HasSuffix(jfrogURL, "jfrog.io") { + t.Skipf("env var JFROG_URL '%s' is not a cloud instance. MyJFrog features are only available on cloud.", jfrogURL) + } + myJFrogAPIToken := os.Getenv("JFROG_MYJFROG_API_TOKEN") + if len(myJFrogAPIToken) == 0 { + t.Fatalf("env var JFROG_MYJFROG_API_TOKEN is not set. Please create a API token in MyJFrog portal") + } + + _, fqrn, allowlistName := testutil.MkNames("test-myjfrog-ip-allowlist", "platform_myjfrog_ip_allowlist") + + re := regexp.MustCompile(`^https://(\w+)\.jfrog\.io$`) + matches := re.FindStringSubmatch(jfrogURL) + if len(matches) < 2 { + t.Fatalf("can't find server name from JFROG_URL %s", jfrogURL) + } + serverName := matches[1] + + temp := ` + resource "platform_myjfrog_ip_allowlist" "{{ .name }}" { + server_name = "{{ .serverName }}" + ips = [] + }` + + testData := map[string]string{ + "name": allowlistName, + "serverName": serverName, + } + + config := testutil.ExecuteTemplate(allowlistName, temp, testData) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProviders(), + Steps: []resource.TestStep{ + { + Config: config, + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(fqrn, "server_name", testData["serverName"]), + resource.TestCheckResourceAttr(fqrn, "ips.#", "0"), + ), + }, + }, + }) +} + +func TestAccIPAllowlist_invalid_ips(t *testing.T) { + jfrogURL := os.Getenv("JFROG_URL") + if !strings.HasSuffix(jfrogURL, "jfrog.io") { + t.Skipf("env var JFROG_URL '%s' is not a cloud instance. MyJFrog features are only available on cloud.", jfrogURL) + } + myJFrogAPIToken := os.Getenv("JFROG_MYJFROG_API_TOKEN") + if len(myJFrogAPIToken) == 0 { + t.Fatalf("env var JFROG_MYJFROG_API_TOKEN is not set. Please create a API token in MyJFrog portal") + } + + re := regexp.MustCompile(`^https://(\w+)\.jfrog\.io$`) + matches := re.FindStringSubmatch(jfrogURL) + if len(matches) < 2 { + t.Fatalf("can't find server name from JFROG_URL %s", jfrogURL) + } + serverName := matches[1] + + for _, invalidIP := range []string{"999.999.999.999", "invalid", "1.1.1.1/99", "999.2.2.2/1"} { + t.Run(invalidIP, func(t *testing.T) { + + _, _, allowlistName := testutil.MkNames("test-myjfrog-ip-allowlist", "platform_myjfrog_ip_allowlist") + + temp := ` + resource "platform_myjfrog_ip_allowlist" "{{ .name }}" { + server_name = "{{ .serverName }}" + ips = ["{{ .invalidIPs }}"] + }` + + testData := map[string]string{ + "name": allowlistName, + "serverName": serverName, + "invalidIPs": invalidIP, + } + + config := testutil.ExecuteTemplate(allowlistName, temp, testData) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProviders(), + Steps: []resource.TestStep{ + { + Config: config, + ExpectError: regexp.MustCompile(`Invalid IP\/CIDR format`), + }, + }, + }) + }) + } +}