From a7cf733be892de0c2b63faf2f89f4f405cb7a89c Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Wed, 6 Sep 2023 12:43:56 -0700 Subject: [PATCH] Add validation to ensure either 'config' or 'path_config' attribute is set Add check for nil pointer before dereferencing --- pkg/xray/resource_xray_repository_config.go | 38 ++++---- .../resource_xray_repository_config_test.go | 95 +++++++++---------- 2 files changed, 65 insertions(+), 68 deletions(-) diff --git a/pkg/xray/resource_xray_repository_config.go b/pkg/xray/resource_xray_repository_config.go index 3f735847..a92a9465 100644 --- a/pkg/xray/resource_xray_repository_config.go +++ b/pkg/xray/resource_xray_repository_config.go @@ -78,11 +78,11 @@ func resourceXrayRepositoryConfig() *schema.Resource { ValidateDiagFunc: validator.StringIsNotEmpty, }, "config": { - Type: schema.TypeSet, - Optional: true, - MaxItems: 1, - Description: "Single repository configuration. Only one of 'config' or 'paths_config' can be set.", - ConflictsWith: []string{"paths_config"}, + Type: schema.TypeSet, + Optional: true, + MaxItems: 1, + Description: "Single repository configuration. Only one of 'config' or 'paths_config' can be set.", + AtLeastOneOf: []string{"paths_config"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "vuln_contextual_analysis": { @@ -141,42 +141,43 @@ func resourceXrayRepositoryConfig() *schema.Resource { }, }, "paths_config": { - Type: schema.TypeSet, - Optional: true, - MaxItems: 1, - Description: `Enables you to set a more granular retention period. It enables you to scan future artifacts within the specific path, and set a retention period for the historical data of artifacts after they are scanned`, + Type: schema.TypeSet, + Optional: true, + MaxItems: 1, + Description: "Enables you to set a more granular retention period. It enables you to scan future artifacts within the specific path, and set a retention period for the historical data of artifacts after they are scanned", + AtLeastOneOf: []string{"config"}, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "pattern": { Type: schema.TypeList, Required: true, MinItems: 1, - Description: `Pattern, applied to the repositories.`, + Description: "Pattern, applied to the repositories.", Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "include": { Type: schema.TypeString, Required: true, - Description: `Include pattern.`, + Description: "Include pattern.", ValidateDiagFunc: validator.StringIsNotEmpty, }, "exclude": { Type: schema.TypeString, Optional: true, - Description: `Exclude pattern.`, + Description: "Exclude pattern.", ValidateDiagFunc: validator.StringIsNotEmpty, }, "index_new_artifacts": { Type: schema.TypeBool, Optional: true, Default: true, - Description: `If checked, Xray will scan newly added artifacts in the path. Note that existing artifacts will not be scanned. If the folder contains existing artifacts that have been scanned, and you do not want to index new artifacts in that folder, you can choose not to index that folder.`, + Description: "If checked, Xray will scan newly added artifacts in the path. Note that existing artifacts will not be scanned. If the folder contains existing artifacts that have been scanned, and you do not want to index new artifacts in that folder, you can choose not to index that folder.", }, "retention_in_days": { Type: schema.TypeInt, Optional: true, Default: 90, - Description: `The artifact will be retained for the number of days you set here, after the artifact is scanned. This will apply to all artifacts in the repository.`, + Description: "The artifact will be retained for the number of days you set here, after the artifact is scanned. This will apply to all artifacts in the repository.", ValidateDiagFunc: validator.IntAtLeast(0), }, }, @@ -185,7 +186,7 @@ func resourceXrayRepositoryConfig() *schema.Resource { "all_other_artifacts": { Type: schema.TypeSet, Required: true, - Description: `If you select by pattern, you must define a retention period for all other artifacts in the repository in the All Other Artifacts setting.`, + Description: "If you select by pattern, you must define a retention period for all other artifacts in the repository in the All Other Artifacts setting.", MinItems: 1, MaxItems: 1, Elem: &schema.Resource{ @@ -194,13 +195,13 @@ func resourceXrayRepositoryConfig() *schema.Resource { Type: schema.TypeBool, Optional: true, Default: true, - Description: `If checked, Xray will scan newly added artifacts in the path. Note that existing artifacts will not be scanned. If the folder contains existing artifacts that have been scanned, and you do not want to index new artifacts in that folder, you can choose not to index that folder.`, + Description: "If checked, Xray will scan newly added artifacts in the path. Note that existing artifacts will not be scanned. If the folder contains existing artifacts that have been scanned, and you do not want to index new artifacts in that folder, you can choose not to index that folder.", }, "retention_in_days": { Type: schema.TypeInt, Optional: true, Default: 90, - Description: `The artifact will be retained for the number of days you set here, after the artifact is scanned. This will apply to all artifacts in the repository.`, + Description: "The artifact will be retained for the number of days you set here, after the artifact is scanned. This will apply to all artifacts in the repository.", ValidateDiagFunc: validator.IntAtLeast(0), }, }, @@ -372,7 +373,7 @@ func resourceXrayRepositoryConfig() *schema.Resource { "retention_in_days": repoConfig.RetentionInDays, } - if slices.Contains(vulnContextualAnalysisPackageTypes(xrayVersion), packageType) { + if repoConfig.VulnContextualAnalysis != nil && slices.Contains(vulnContextualAnalysisPackageTypes(xrayVersion), packageType) { m["vuln_contextual_analysis"] = *repoConfig.VulnContextualAnalysis } @@ -528,5 +529,4 @@ func resourceXrayRepositoryConfig() *schema.Resource { Schema: repositoryConfigSchema, Description: "Provides an Xray repository config resource. See [Xray Indexing Resources](https://www.jfrog.com/confluence/display/JFROG/Indexing+Xray+Resources#IndexingXrayResources-SetaRetentionPeriod) and [REST API](https://www.jfrog.com/confluence/display/JFROG/Xray+REST+API#XrayRESTAPI-UpdateRepositoriesConfigurations) for more details.", } - } diff --git a/pkg/xray/resource_xray_repository_config_test.go b/pkg/xray/resource_xray_repository_config_test.go index 5df1e862..b4b84e6c 100644 --- a/pkg/xray/resource_xray_repository_config_test.go +++ b/pkg/xray/resource_xray_repository_config_test.go @@ -11,32 +11,29 @@ import ( "github.com/jfrog/terraform-provider-shared/util/sdk" ) -func TestAccRepositoryConfigRepoConfigNegative(t *testing.T) { +func TestAccRepositoryConfigRepoNoConfig(t *testing.T) { _, fqrn, resourceName := testutil.MkNames("xray-repo-config-", "xray_repository_config") var testData = map[string]string{ - "resource_name": resourceName, - "repo_name": "repo-config-test-repo", - "retention_in_days": "90", - "pattern0_include": "core/**", - "pattern0_exclude": "core/external/**", - "pattern0_index_new_artifacts": "true", - "pattern0_retention_in_days": "45", - "pattern1_include": "core/**", - "pattern1_exclude": "core/external/**", - "pattern1_index_new_artifacts": "true", - "pattern1_retention_in_days": "45", - "other_index_new_artifacts": "true", - "other_retention_in_days": "60", + "resource_name": resourceName, + "repo_name": "repo-config-test-repo", } + config := sdk.ExecuteTemplate( + fqrn, + `resource "xray_repository_config" "{{ .resource_name }}" { + repo_name = "{{ .repo_name }}" + }`, + testData, + ) + resource.Test(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, ProviderFactories: testAccProviders(), Steps: []resource.TestStep{ { - Config: sdk.ExecuteTemplate(fqrn, TestDataRepoConfigErrorTemplate, testData), - ExpectError: regexp.MustCompile("Conflicting configuration arguments"), + Config: config, + ExpectError: regexp.MustCompile("Missing required argument"), }, }, }) @@ -115,7 +112,7 @@ func testAccRepositoryConfigRepoConfigCreate_VulnContextualAnalysis(t *testing.T } } -func TestAccRepositoryConfigRepoConfigCreate_Exposure(t *testing.T) { +func TestAccRepositoryConfigRepoConfigCreate_exposure(t *testing.T) { testCase := []struct { packageType string template string @@ -179,6 +176,37 @@ func TestAccRepositoryConfigRepoConfigCreate_Exposure(t *testing.T) { } } +func TestAccRepositoryConfigRepoConfigCreate_no_exposure(t *testing.T) { + packageTypes := []string{"alpine", "bower", "composer", "conan", "conda", "debian", "gems", "generic", "go", "gradle", "ivy", "nuget", "rpm", "sbt"} + template := ` + resource "xray_repository_config" "{{ .resource_name }}" { + repo_name = "{{ .repo_name }}" + + config { + retention_in_days = {{ .retention_in_days }} + } + }` + validVersion := "3.75.10" + version, err := sdk.GetXrayVersion(GetTestResty(t)) + if err != nil { + t.Fail() + return + } + + checkFunc := func(fqrn string, testData map[string]string) resource.TestCheckFunc { + return resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr(fqrn, "repo_name", testData["repo_name"]), + resource.TestCheckResourceAttr(fqrn, "config.0.retention_in_days", testData["retention_in_days"]), + resource.TestCheckNoResourceAttr(fqrn, "config.0.vuln_contextual_analysis"), + resource.TestCheckResourceAttr(fqrn, "config.0.exposures.#", "0"), + ) + } + + for _, packageType := range packageTypes { + t.Run(packageType, testAccRepositoryConfigRepoConfigCreate(t, packageType, template, validVersion, version, checkFunc)) + } +} + func testAccRepositoryConfigRepoConfigCreate(t *testing.T, packageType, template, validVersion, xrayVersion string, checkFunc func(fqrn string, testData map[string]string) resource.TestCheckFunc) func(t *testing.T) { return func(t *testing.T) { _, fqrn, resourceName := testutil.MkNames("xray-repo-config-", "xray_repository_config") @@ -211,7 +239,6 @@ func testAccRepositoryConfigRepoConfigCreate(t *testing.T, packageType, template return errorResp, err }), ProviderFactories: testAccProviders(), - Steps: []resource.TestStep{ { Config: sdk.ExecuteTemplate(fqrn, template, testData), @@ -377,36 +404,6 @@ func verifyRepositoryConfig(fqrn string, testData map[string]string) resource.Te ) } -const TestDataRepoConfigErrorTemplate = ` -resource "xray_repository_config" "{{ .resource_name }}" { - repo_name = "{{ .repo_name }}" - - config { - retention_in_days = {{ .retention_in_days }} - } - - paths_config { - pattern { - include = "{{ .pattern0_include }}" - exclude = "{{ .pattern0_exclude }}" - index_new_artifacts = {{ .pattern0_index_new_artifacts }} - retention_in_days = {{ .pattern0_retention_in_days }} - } - - pattern { - include = "{{ .pattern1_include }}" - exclude = "{{ .pattern1_exclude }}" - index_new_artifacts = {{ .pattern1_index_new_artifacts }} - retention_in_days = {{ .pattern1_retention_in_days }} - } - - all_other_artifacts { - index_new_artifacts = {{ .other_index_new_artifacts }} - retention_in_days = {{ .other_retention_in_days }} - } - } -}` - const TestDataRepoConfigDockerTemplate = ` resource "xray_repository_config" "{{ .resource_name }}" { repo_name = "{{ .repo_name }}" @@ -446,7 +443,7 @@ resource "xray_repository_config" "{{ .resource_name }}" { repo_name = "{{ .repo_name }}" config { - retention_in_days = {{ .retention_in_days }} + retention_in_days = {{ .retention_in_days }} exposures { scanners_category {