From 6a8c79e1b37ab9fca0955fae34ee96dfa7816d7c Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Thu, 23 May 2024 08:52:25 -0700 Subject: [PATCH 1/3] Fix doc for TFC example --- docs/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/index.md b/docs/index.md index 973f03bb..5329c1cc 100644 --- a/docs/index.md +++ b/docs/index.md @@ -128,7 +128,7 @@ terraform { } } -provider "platform" { +provider "artifactory" { url = "https://myinstance.jfrog.io" oidc_provider_name = "terraform-cloud" } From 106c1bad32cec0d54439103a2885c715ab20de3b Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Thu, 23 May 2024 12:38:49 -0700 Subject: [PATCH 2/3] Fix various crashes when importing from scratch With optional attributes omitted in configuration. --- .../resource_artifactory_custom_webhook.go | 38 ++++-- .../webhook/resource_artifactory_webhook.go | 58 ++++++--- .../resource_artifactory_webhook_repo.go | 10 +- .../resource_artifactory_webhook_test.go | 114 ++++++++++++------ 4 files changed, 156 insertions(+), 64 deletions(-) diff --git a/pkg/artifactory/resource/webhook/resource_artifactory_custom_webhook.go b/pkg/artifactory/resource/webhook/resource_artifactory_custom_webhook.go index 72105462..fff27d6d 100644 --- a/pkg/artifactory/resource/webhook/resource_artifactory_custom_webhook.go +++ b/pkg/artifactory/resource/webhook/resource_artifactory_custom_webhook.go @@ -174,11 +174,24 @@ func ResourceArtifactoryCustomWebhook(webhookType string) *schema.Resource { webhookHandler := CustomHandler{ HandlerType: "custom-webhook", Url: h["url"].(string), - Secrets: unpackKeyValuePair(h["secrets"].(map[string]interface{})), - Proxy: h["proxy"].(string), - HttpHeaders: unpackKeyValuePair(h["http_headers"].(map[string]interface{})), - Payload: h["payload"].(string), } + + if v, ok := h["secrets"]; ok { + webhookHandler.Secrets = unpackKeyValuePair(v.(map[string]interface{})) + } + + if v, ok := h["proxy"]; ok { + webhookHandler.Proxy = v.(string) + } + + if v, ok := h["http_headers"]; ok { + webhookHandler.HttpHeaders = unpackKeyValuePair(v.(map[string]interface{})) + } + + if v, ok := h["payload"]; ok { + webhookHandler.Payload = v.(string) + } + webhookHandlers = append(webhookHandlers, webhookHandler) } } @@ -208,12 +221,19 @@ func ResourceArtifactoryCustomWebhook(webhookType string) *schema.Resource { packedHandlers := make([]interface{}, len(handlers)) for _, handler := range handlers { packedHandler := map[string]interface{}{ - "url": handler.Url, - "secrets": packSecretsCustom(handler.Secrets, d, handler.Url), - "proxy": handler.Proxy, - "http_headers": packKeyValuePair(handler.HttpHeaders), - "payload": handler.Payload, + "url": handler.Url, + "proxy": handler.Proxy, + "payload": handler.Payload, + } + + if handler.Secrets != nil { + packedHandler["secrets"] = packSecretsCustom(handler.Secrets, d, handler.Url) } + + if handler.HttpHeaders != nil { + packedHandler["http_headers"] = packKeyValuePair(handler.HttpHeaders) + } + packedHandlers = append(packedHandlers, packedHandler) } diff --git a/pkg/artifactory/resource/webhook/resource_artifactory_webhook.go b/pkg/artifactory/resource/webhook/resource_artifactory_webhook.go index e2e84ecc..2c4f012b 100644 --- a/pkg/artifactory/resource/webhook/resource_artifactory_webhook.go +++ b/pkg/artifactory/resource/webhook/resource_artifactory_webhook.go @@ -77,25 +77,25 @@ const WhUrl = webhooksUrl + "/{webhookKey}" const currentSchemaVersion = 2 var unpackKeyValuePair = func(keyValuePairs map[string]interface{}) []KeyValuePair { - var KVPairs []KeyValuePair + var kvPairs []KeyValuePair for key, value := range keyValuePairs { keyValuePair := KeyValuePair{ Name: key, Value: value.(string), } - KVPairs = append(KVPairs, keyValuePair) + kvPairs = append(kvPairs, keyValuePair) } - return KVPairs + return kvPairs } var packKeyValuePair = func(keyValuePairs []KeyValuePair) map[string]interface{} { - KVPairs := make(map[string]interface{}) + kvPairs := make(map[string]interface{}) for _, keyValuePair := range keyValuePairs { - KVPairs[keyValuePair.Name] = keyValuePair.Value + kvPairs[keyValuePair.Name] = keyValuePair.Value } - return KVPairs + return kvPairs } var domainCriteriaLookup = map[string]interface{}{ @@ -166,8 +166,17 @@ var packCriteria = func(d *schema.ResourceData, webhookType string, criteria map resource := domainSchemaLookup(currentSchemaVersion, false, webhookType)[webhookType]["criteria"].Elem.(*schema.Resource) packedCriteria := domainPackLookup[webhookType](criteria) - packedCriteria["include_patterns"] = schema.NewSet(schema.HashString, criteria["includePatterns"].([]interface{})) - packedCriteria["exclude_patterns"] = schema.NewSet(schema.HashString, criteria["excludePatterns"].([]interface{})) + includePatterns := []interface{}{} + if v, ok := criteria["includePatterns"]; ok && v != nil { + includePatterns = v.([]interface{}) + } + packedCriteria["include_patterns"] = schema.NewSet(schema.HashString, includePatterns) + + excludePatterns := []interface{}{} + if v, ok := criteria["excludePatterns"]; ok && v != nil { + excludePatterns = v.([]interface{}) + } + packedCriteria["exclude_patterns"] = schema.NewSet(schema.HashString, excludePatterns) return setValue("criteria", schema.NewSet(schema.HashResource(resource), []interface{}{packedCriteria})) } @@ -190,7 +199,7 @@ var packSecret = func(d *schema.ResourceData, url string) string { for _, handler := range handlers { h := handler.(map[string]interface{}) // if urls match, assign the secret value from the state - if h["url"] == url { + if h["url"].(string) == url { secret = h["secret"].(string) } } @@ -215,13 +224,26 @@ func ResourceArtifactoryWebhook(webhookType string) *schema.Resource { // https://discuss.hashicorp.com/t/using-typeset-in-provider-always-adds-an-empty-element-on-update/18566/2 if h["url"].(string) != "" { webhookHandler := Handler{ - HandlerType: "webhook", - Url: h["url"].(string), - Secret: h["secret"].(string), - UseSecretForSigning: h["use_secret_for_signing"].(bool), - Proxy: h["proxy"].(string), - CustomHttpHeaders: unpackKeyValuePair(h["custom_http_headers"].(map[string]interface{})), + HandlerType: "webhook", + Url: h["url"].(string), + } + + if v, ok := h["secret"]; ok { + webhookHandler.Secret = v.(string) + } + + if v, ok := h["use_secret_for_signing"]; ok { + webhookHandler.UseSecretForSigning = v.(bool) } + + if v, ok := h["proxy"]; ok { + webhookHandler.Proxy = v.(string) + } + + if v, ok := h["custom_http_headers"]; ok { + webhookHandler.CustomHttpHeaders = unpackKeyValuePair(v.(map[string]interface{})) + } + webhookHandlers = append(webhookHandlers, webhookHandler) } } @@ -255,8 +277,12 @@ func ResourceArtifactoryWebhook(webhookType string) *schema.Resource { "secret": packSecret(d, handler.Url), "use_secret_for_signing": handler.UseSecretForSigning, "proxy": handler.Proxy, - "custom_http_headers": packKeyValuePair(handler.CustomHttpHeaders), } + + if handler.CustomHttpHeaders != nil { + packedHandler["custom_http_headers"] = packKeyValuePair(handler.CustomHttpHeaders) + } + packedHandlers = append(packedHandlers, packedHandler) } diff --git a/pkg/artifactory/resource/webhook/resource_artifactory_webhook_repo.go b/pkg/artifactory/resource/webhook/resource_artifactory_webhook_repo.go index dbb53f40..93e9e354 100644 --- a/pkg/artifactory/resource/webhook/resource_artifactory_webhook_repo.go +++ b/pkg/artifactory/resource/webhook/resource_artifactory_webhook_repo.go @@ -54,12 +54,18 @@ var repoWebhookSchema = func(webhookType string, version int, isCustom bool) map } var packRepoCriteria = func(artifactoryCriteria map[string]interface{}) map[string]interface{} { - return map[string]interface{}{ + criteria := map[string]interface{}{ "any_local": artifactoryCriteria["anyLocal"].(bool), "any_remote": artifactoryCriteria["anyRemote"].(bool), - "any_federated": artifactoryCriteria["anyFederated"].(bool), + "any_federated": false, "repo_keys": schema.NewSet(schema.HashString, artifactoryCriteria["repoKeys"].([]interface{})), } + + if v, ok := artifactoryCriteria["anyFederated"]; ok { + criteria["any_federated"] = v.(bool) + } + + return criteria } var unpackRepoCriteria = func(terraformCriteria map[string]interface{}, baseCriteria BaseWebhookCriteria) interface{} { diff --git a/pkg/artifactory/resource/webhook/resource_artifactory_webhook_test.go b/pkg/artifactory/resource/webhook/resource_artifactory_webhook_test.go index f2eb3eac..b62a132a 100644 --- a/pkg/artifactory/resource/webhook/resource_artifactory_webhook_test.go +++ b/pkg/artifactory/resource/webhook/resource_artifactory_webhook_test.go @@ -80,7 +80,7 @@ var releaseBundleTemplate = ` } ` -func TestAccWebhookCriteriaValidation(t *testing.T) { +func TestAccWebhook_CriteriaValidation(t *testing.T) { for _, webhookType := range webhook.TypesSupported { t.Run(webhookType, func(t *testing.T) { resource.Test(webhookCriteriaValidationTestCase(webhookType, t)) @@ -124,7 +124,7 @@ func webhookCriteriaValidationTestCase(webhookType string, t *testing.T) (*testi } } -func TestAccWebhookEventTypesValidation(t *testing.T) { +func TestAccWebhook_EventTypesValidation(t *testing.T) { id := testutil.RandomInt() name := fmt.Sprintf("webhook-%d", id) fqrn := fmt.Sprintf("artifactory_artifact_webhook.%s", name) @@ -166,7 +166,7 @@ func TestAccWebhookEventTypesValidation(t *testing.T) { }) } -func TestAccWebhookHandlerValidation_EmptyProxy(t *testing.T) { +func TestAccWebhook_HandlerValidation_EmptyProxy(t *testing.T) { id := testutil.RandomInt() name := fmt.Sprintf("webhook-%d", id) fqrn := fmt.Sprintf("artifactory_artifact_webhook.%s", name) @@ -206,7 +206,7 @@ func TestAccWebhookHandlerValidation_EmptyProxy(t *testing.T) { }) } -func TestAccWebhookHandlerValidation_ProxyWithURL(t *testing.T) { +func TestAccWebhook_HandlerValidation_ProxyWithURL(t *testing.T) { id := testutil.RandomInt() name := fmt.Sprintf("webhook-%d", id) fqrn := fmt.Sprintf("artifactory_artifact_webhook.%s", name) @@ -246,7 +246,7 @@ func TestAccWebhookHandlerValidation_ProxyWithURL(t *testing.T) { }) } -func TestAccWebhookAllTypes(t *testing.T) { +func TestAccWebhook_AllTypes(t *testing.T) { // Can only realistically test these 3 types of webhook since creating // build, release_bundle, or distribution in test environment is almost impossible for _, webhookType := range []string{"artifact", "artifact_property", "docker"} { @@ -256,7 +256,7 @@ func TestAccWebhookAllTypes(t *testing.T) { } } -func TestAccCustomWebhookAllTypes(t *testing.T) { +func TestAccCustomWebhook_AllTypes(t *testing.T) { // Can only realistically test these 3 types of webhook since creating // build, release_bundle, or distribution in test environment is almost impossible for _, webhookType := range []string{"artifact", "artifact_property", "docker"} { @@ -313,13 +313,40 @@ func webhookTestCase(webhookType string, t *testing.T) (*testing.T, resource.Tes } } handler { - url = "https://tempurl.com" - secret = "fake-secret-2" + url = "https://tempurl.com" + } + + depends_on = [artifactory_local_{{ .repoType }}_repository.{{ .repoName }}] + } + `, params) + + updatedConfig := util.ExecuteTemplate("TestAccWebhook{{ .webhookType }}Type", ` + resource "artifactory_local_{{ .repoType }}_repository" "{{ .repoName }}" { + key = "{{ .repoName }}" + } + + resource "artifactory_{{ .webhookType }}_webhook" "{{ .webhookName }}" { + key = "{{ .webhookName }}" + description = "test description" + event_types = [{{ range $index, $eventType := .eventTypes}}{{if $index}},{{end}}"{{$eventType}}"{{end}}] + criteria { + any_local = {{ .anyLocal }} + any_remote = {{ .anyRemote }} + any_federated = {{ .anyFederated }} + repo_keys = ["{{ .repoName }}"] + } + handler { + url = "https://tempurl.org" + secret = "fake-secret" + use_secret_for_signing = {{ .useSecretForSigning }} custom_http_headers = { - header-3 = "value-3" - header-4 = "value-4" + header-1 = "value-1" + header-2 = "value-2" } } + handler { + url = "https://tempurl.com" + } depends_on = [artifactory_local_{{ .repoType }}_repository.{{ .repoName }}] } @@ -345,10 +372,30 @@ func webhookTestCase(webhookType string, t *testing.T) (*testing.T, resource.Tes resource.TestCheckResourceAttr(fqrn, "handler.0.custom_http_headers.header-1", "value-1"), resource.TestCheckResourceAttr(fqrn, "handler.0.custom_http_headers.header-2", "value-2"), resource.TestCheckResourceAttr(fqrn, "handler.1.url", "https://tempurl.com"), - resource.TestCheckResourceAttr(fqrn, "handler.1.secret", "fake-secret-2"), - resource.TestCheckResourceAttr(fqrn, "handler.1.custom_http_headers.%", "2"), - resource.TestCheckResourceAttr(fqrn, "handler.1.custom_http_headers.header-3", "value-3"), - resource.TestCheckResourceAttr(fqrn, "handler.1.custom_http_headers.header-4", "value-4"), + resource.TestCheckResourceAttr(fqrn, "handler.1.secret", ""), + resource.TestCheckNoResourceAttr(fqrn, "handler.1.custom_http_headers"), + } + + updatedTestChecks := []resource.TestCheckFunc{ + resource.TestCheckResourceAttr(fqrn, "key", name), + resource.TestCheckResourceAttr(fqrn, "event_types.#", fmt.Sprintf("%d", len(eventTypes))), + resource.TestCheckResourceAttr(fqrn, "criteria.#", "1"), + resource.TestCheckResourceAttr(fqrn, "criteria.0.any_local", fmt.Sprintf("%t", params["anyLocal"])), + resource.TestCheckResourceAttr(fqrn, "criteria.0.any_remote", fmt.Sprintf("%t", params["anyRemote"])), + resource.TestCheckResourceAttr(fqrn, "criteria.0.any_federated", fmt.Sprintf("%t", params["anyFederated"])), + resource.TestCheckTypeSetElemAttr(fqrn, "criteria.0.repo_keys.*", repoName), + resource.TestCheckResourceAttr(fqrn, "criteria.0.include_patterns.#", "0"), + resource.TestCheckResourceAttr(fqrn, "criteria.0.exclude_patterns.#", "0"), + resource.TestCheckResourceAttr(fqrn, "handler.#", "2"), + resource.TestCheckResourceAttr(fqrn, "handler.0.url", "https://tempurl.org"), + resource.TestCheckResourceAttr(fqrn, "handler.0.secret", "fake-secret"), + resource.TestCheckResourceAttr(fqrn, "handler.0.use_secret_for_signing", fmt.Sprintf("%t", params["useSecretForSigning"])), + resource.TestCheckResourceAttr(fqrn, "handler.0.custom_http_headers.%", "2"), + resource.TestCheckResourceAttr(fqrn, "handler.0.custom_http_headers.header-1", "value-1"), + resource.TestCheckResourceAttr(fqrn, "handler.0.custom_http_headers.header-2", "value-2"), + resource.TestCheckResourceAttr(fqrn, "handler.1.url", "https://tempurl.com"), + resource.TestCheckResourceAttr(fqrn, "handler.1.secret", ""), + resource.TestCheckResourceAttr(fqrn, "handler.1.custom_http_headers.#", "0"), } for _, eventType := range eventTypes { @@ -366,12 +413,16 @@ func webhookTestCase(webhookType string, t *testing.T) (*testing.T, resource.Tes Config: webhookConfig, Check: resource.ComposeTestCheckFunc(testChecks...), }, + { + Config: updatedConfig, + Check: resource.ComposeTestCheckFunc(updatedTestChecks...), + }, { ResourceName: fqrn, ImportState: true, ImportStateVerify: true, ImportStateCheck: validator.CheckImportState(name, "key"), - ImportStateVerifyIgnore: []string{"handler.0.secret", "handler.1.secret"}, + ImportStateVerifyIgnore: []string{"handler.0.secret"}, }, }, } @@ -414,8 +465,8 @@ func customWebhookTestCase(webhookType string, t *testing.T) (*testing.T, resour exclude_patterns = ["bar/**"] } handler { - url = "https://tempurl.org" - secrets = { + url = "https://tempurl.org" + secrets = { secret1 = "value1" secret2 = "value2" } @@ -426,8 +477,8 @@ func customWebhookTestCase(webhookType string, t *testing.T) (*testing.T, resour payload = "{ \"ref\": \"main\" , \"inputs\": { \"artifact_path\": \"test-repo/repo-path\" } }" } handler { - url = "https://tempurl.org" - secrets = { + url = "https://tempurl.org" + secrets = { secret3 = "value3" secret4 = "value4" } @@ -438,15 +489,7 @@ func customWebhookTestCase(webhookType string, t *testing.T) (*testing.T, resour payload = "{ \"ref\": \"main\" , \"inputs\": { \"artifact_path\": \"test-repo/repo-path\" } }" } handler { - url = "https://tempurl.com" - secrets = { - secret5 = "value5" - secret6 = "value6" - } - http_headers = { - header-5 = "value-5" - header-6 = "value-6" - } + url = "https://tempurl.com" payload = "{ \"ref\": \"main\" , \"inputs\": { \"artifact_path\": \"test-repo/repo-path\" } }" } @@ -484,12 +527,8 @@ func customWebhookTestCase(webhookType string, t *testing.T) (*testing.T, resour resource.TestCheckResourceAttr(fqrn, "handler.1.http_headers.header-4", "value-4"), resource.TestCheckResourceAttr(fqrn, "handler.1.payload", "{ \"ref\": \"main\" , \"inputs\": { \"artifact_path\": \"test-repo/repo-path\" } }"), resource.TestCheckResourceAttr(fqrn, "handler.2.url", "https://tempurl.com"), - resource.TestCheckResourceAttr(fqrn, "handler.2.secrets.%", "2"), - resource.TestCheckResourceAttr(fqrn, "handler.2.secrets.secret5", "value5"), - resource.TestCheckResourceAttr(fqrn, "handler.2.secrets.secret6", "value6"), - resource.TestCheckResourceAttr(fqrn, "handler.2.http_headers.%", "2"), - resource.TestCheckResourceAttr(fqrn, "handler.2.http_headers.header-5", "value-5"), - resource.TestCheckResourceAttr(fqrn, "handler.2.http_headers.header-6", "value-6"), + resource.TestCheckNoResourceAttr(fqrn, "handler.2.secrets"), + resource.TestCheckNoResourceAttr(fqrn, "handler.2.http_headers"), resource.TestCheckResourceAttr(fqrn, "handler.2.payload", "{ \"ref\": \"main\" , \"inputs\": { \"artifact_path\": \"test-repo/repo-path\" } }"), } @@ -513,7 +552,7 @@ func customWebhookTestCase(webhookType string, t *testing.T) (*testing.T, resour ImportState: true, ImportStateVerify: true, ImportStateCheck: validator.CheckImportState(name, "key"), - ImportStateVerifyIgnore: []string{"handler.0.secrets", "handler.1.secrets", "handler.2.secrets"}, + ImportStateVerifyIgnore: []string{"handler.0.secrets", "handler.1.secrets"}, }, }, } @@ -525,7 +564,8 @@ func testCheckWebhook(id string, request *resty.Request) (*resty.Response, error AddRetryCondition(client.NeverRetry). Get(webhook.WhUrl) } -func TestGH476WebHookChangeBearerSet0(t *testing.T) { + +func TestAccWebhook_GH476WebHookChangeBearerSet0(t *testing.T) { _, fqrn, name := testutil.MkNames("foo", "artifactory_artifact_webhook") format := ` @@ -607,7 +647,7 @@ func TestGH476WebHookChangeBearerSet0(t *testing.T) { } // Unit tests for state migration func -func TestWebhookResourceStateUpgradeV1(t *testing.T) { +func TestWebhook_ResourceStateUpgradeV1(t *testing.T) { v1Data := map[string]interface{}{ "url": "https://tempurl.org", "secret": "fake-secret", From 515b6795bd96750908e14ebdbe4fb787be722b02 Mon Sep 17 00:00:00 2001 From: Alex Hung Date: Thu, 23 May 2024 13:10:03 -0700 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3464595f..2b550fe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 10.8.1 (May 24, 2024) + +BUG FIXES: + +* resource/artifactory_\*\_webhook, resource/artifactory_\*\_custom_webhook: Fix various crashes when importing the resource with optional attributes not set in the configuration. PR: [#973](https://github.com/jfrog/terraform-provider-artifactory/pull/973) + ## 10.8.0 (May 20, 2024) IMPROVEMENTS: