diff --git a/CHANGELOG.md b/CHANGELOG.md index 19e65a3..a77de47 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,9 @@ +## 2.5.4 + +BUG FIXES: + +- Fix `skip_tls_verify` being ignored unless cert-related attributes were also set, restoring v1.x behavior for self-signed certificates. Closes #128. + ## 2.5.3 BUG FIXES: diff --git a/internal/provider/curl_action.go b/internal/provider/curl_action.go index 94426c3..66a7a9a 100644 --- a/internal/provider/curl_action.go +++ b/internal/provider/curl_action.go @@ -146,17 +146,9 @@ func (c *CurlAction) Invoke(ctx context.Context, req action.InvokeRequest, resp return } - useTLS := !data.CertFile.IsNull() || !data.KeyFile.IsNull() || !data.CaCertFile.IsNull() || !data.CaCertDirectory.IsNull() - var tlsConfig *TlsConfig - if useTLS { - tlsConfig = &TlsConfig{ - CertFile: data.CertFile.ValueString(), - KeyFile: data.KeyFile.ValueString(), - CaCertFile: data.CaCertFile.ValueString(), - CaCertDirectory: data.CaCertDirectory.ValueString(), - SkipTlsVerify: data.SkipTlsVerify.ValueBool(), - } + if needsTlsClient(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) { + tlsConfig = tlsConfigFromAttrs(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) if tlsConfig.CertFile != "" && tlsConfig.KeyFile == "" { resp.Diagnostics.AddError("Validation Error", "`key_file` must be set if `cert_file` is set.") diff --git a/internal/provider/curl_data_source.go b/internal/provider/curl_data_source.go index c7c1ef1..6a7e3a8 100644 --- a/internal/provider/curl_data_source.go +++ b/internal/provider/curl_data_source.go @@ -187,20 +187,12 @@ func (d *CurlDataSource) Read(ctx context.Context, req datasource.ReadRequest, r data.ID = types.StringValue(data.Name.ValueString()) - useTLS := !data.CertFile.IsNull() || !data.KeyFile.IsNull() || !data.CaCertFile.IsNull() || !data.CaCertDirectory.IsNull() - var client *http.Client var err error var tlsConfig *TlsConfig - if useTLS { - tlsConfig = &TlsConfig{ - CertFile: data.CertFile.ValueString(), - KeyFile: data.KeyFile.ValueString(), - CaCertFile: data.CaCertFile.ValueString(), - CaCertDirectory: data.CaCertDirectory.ValueString(), - SkipTlsVerify: data.SkipTlsVerify.ValueBool(), - } + if needsTlsClient(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) { + tlsConfig = tlsConfigFromAttrs(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) if tlsConfig.CertFile != "" && tlsConfig.KeyFile == "" { resp.Diagnostics.AddError("Validation Error", "`key_file` must be set if `cert_file` is set.") diff --git a/internal/provider/curl_data_source_test.go b/internal/provider/curl_data_source_test.go index 4fdd17b..7fdfc62 100644 --- a/internal/provider/curl_data_source_test.go +++ b/internal/provider/curl_data_source_test.go @@ -330,6 +330,49 @@ data "terracurl_request" "tls_skip_verify_test" { `, url, certFile, keyFile) } +func TestAccCurlDataSourceSkipTlsVerifyOnly(t *testing.T) { + t.Setenv("TF_ACC", "true") + t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") + + server, certFile, keyFile, err := createTLSServer() + if err != nil { + t.Fatalf("failed to create TLS test server: %v", err) + } + defer server.Close() + defer func(name string) { + _ = os.Remove(name) + }(certFile) + defer func(name string) { + _ = os.Remove(name) + }(keyFile) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceCurlSkipTlsVerifyOnly(server.URL), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("data.terracurl_request.skip_tls_only", "method", "GET"), + resource.TestCheckResourceAttr("data.terracurl_request.skip_tls_only", "response", `{"message": "TLS test successful"}`), + ), + }, + }, + }) +} + +func testAccDataSourceCurlSkipTlsVerifyOnly(url string) string { + return fmt.Sprintf(` +data "terracurl_request" "skip_tls_only" { + name = "skip-tls-only" + method = "GET" + url = "%s" + response_codes = ["200"] + skip_tls_verify = true +} +`, url) +} + func TestAccDataSourceCurlResponseSensitive(t *testing.T) { t.Setenv("TF_ACC", "true") t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") diff --git a/internal/provider/curl_ephemeral_resource.go b/internal/provider/curl_ephemeral_resource.go index 82cf30b..035e72d 100644 --- a/internal/provider/curl_ephemeral_resource.go +++ b/internal/provider/curl_ephemeral_resource.go @@ -425,22 +425,13 @@ func (e *EphemeralCurlResource) Open(ctx context.Context, req ephemeral.OpenRequ data.Id = types.StringValue(data.Name.ValueString()) - // useTLS is used to decide. - useTLS := !data.CertFile.IsNull() || !data.KeyFile.IsNull() || !data.CaCertFile.IsNull() || !data.CaCertDirectory.IsNull() - var client *http.Client var err error var tlsConfig *TlsConfig - if useTLS { + if needsTlsClient(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) { tflog.Debug(ctx, "Creating TLS enabled client") - tlsConfig = &TlsConfig{ - CertFile: data.CertFile.ValueString(), - KeyFile: data.KeyFile.ValueString(), - CaCertFile: data.CaCertFile.ValueString(), - CaCertDirectory: data.CaCertDirectory.ValueString(), - SkipTlsVerify: data.SkipTlsVerify.ValueBool(), - } + tlsConfig = tlsConfigFromAttrs(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) if tlsConfig.CertFile != "" && tlsConfig.KeyFile == "" { resp.Diagnostics.AddError("Validation Error", "`key_file` must be set if `cert_file` is set.") @@ -1070,9 +1061,11 @@ func (e *EphemeralCurlResource) Renew(ctx context.Context, req ephemeral.RenewRe var client *http.Client - useTls := (!privateData.RenewCertFile.IsNull() && privateData.RenewCertFile.ValueString() != "") || - (!privateData.RenewKeyFile.IsNull() && privateData.RenewKeyFile.ValueString() != "") || - (!privateData.RenewCaCertFile.IsNull() && privateData.RenewCaCertFile.ValueString() != "") + useTls := hasValue(privateData.RenewCertFile) || + hasValue(privateData.RenewKeyFile) || + hasValue(privateData.RenewCaCertFile) || + hasValue(privateData.RenewCaCertDirectory) || + privateData.RenewSkipTlsVerify.ValueBool() var tlsConfig *TlsConfig if useTls { @@ -1420,7 +1413,9 @@ func (e *EphemeralCurlResource) Close(ctx context.Context, req ephemeral.CloseRe var client *http.Client useCloseTls := hasValue(privateData.CloseCertFile) || hasValue(privateData.CloseKeyFile) || - hasValue(privateData.CloseCaCertFile) + hasValue(privateData.CloseCaCertFile) || + hasValue(privateData.CloseCaCertDirectory) || + privateData.CloseSkipTlsVerify.ValueBool() var closeTlsConfig *TlsConfig if useCloseTls { diff --git a/internal/provider/curl_resource.go b/internal/provider/curl_resource.go index 5dcd171..c5089bf 100644 --- a/internal/provider/curl_resource.go +++ b/internal/provider/curl_resource.go @@ -488,21 +488,12 @@ func (r *CurlResource) Create(ctx context.Context, req resource.CreateRequest, r data.Id = types.StringValue(data.Name.ValueString()) - // useTLS is used to decide - useTLS := !data.CertFile.IsNull() || !data.KeyFile.IsNull() || !data.CaCertFile.IsNull() || !data.CaCertDirectory.IsNull() - var client *http.Client var err error var tlsConfig *TlsConfig - if useTLS { - tlsConfig = &TlsConfig{ - CertFile: data.CertFile.ValueString(), - KeyFile: data.KeyFile.ValueString(), - CaCertFile: data.CaCertFile.ValueString(), - CaCertDirectory: data.CaCertDirectory.ValueString(), - SkipTlsVerify: data.SkipTlsVerify.ValueBool(), - } + if needsTlsClient(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) { + tlsConfig = tlsConfigFromAttrs(data.CertFile, data.KeyFile, data.CaCertFile, data.CaCertDirectory, data.SkipTlsVerify) if tlsConfig.CertFile != "" && tlsConfig.KeyFile == "" { resp.Diagnostics.AddError("Validation Error", "`key_file` must be set if `cert_file` is set.") @@ -630,19 +621,10 @@ func ignoredResponseFields(data CurlResourceModel) []string { } func (r *CurlResource) executeReadRequest(ctx context.Context, data CurlResourceModel) (statusCode int, body string, diags diag.Diagnostics) { - useReadTls := !data.ReadCertFile.IsNull() || !data.ReadKeyFile.IsNull() || !data.ReadCaCertFile.IsNull() - var readTlsConfig *TlsConfig - if useReadTls { + if needsTlsClient(data.ReadCertFile, data.ReadKeyFile, data.ReadCaCertFile, data.ReadCaCertDirectory, data.ReadSkipTlsVerify) { tflog.Debug(ctx, "Using custom TLS client for Read() operation") - - readTlsConfig = &TlsConfig{ - CertFile: data.ReadCertFile.ValueString(), - KeyFile: data.ReadKeyFile.ValueString(), - CaCertFile: data.ReadCaCertFile.ValueString(), - CaCertDirectory: data.ReadCaCertDirectory.ValueString(), - SkipTlsVerify: data.ReadSkipTlsVerify.ValueBool(), - } + readTlsConfig = tlsConfigFromAttrs(data.ReadCertFile, data.ReadKeyFile, data.ReadCaCertFile, data.ReadCaCertDirectory, data.ReadSkipTlsVerify) } else { tflog.Debug(ctx, "Using default HTTP client for Read() operation") } @@ -864,19 +846,10 @@ func (r *CurlResource) Delete(ctx context.Context, req resource.DeleteRequest, r // Build TLS Client if `destroy_*` TLS Arguments Provided var client *http.Client - useDestroyTls := !data.DestroyCertFile.IsNull() || !data.DestroyKeyFile.IsNull() || !data.DestroyCaCertFile.IsNull() - var destroyTlsConfig *TlsConfig - if useDestroyTls { + if needsTlsClient(data.DestroyCertFile, data.DestroyKeyFile, data.DestroyCaCertFile, data.DestroyCaCertDirectory, data.DestroySkipTlsVerify) { tflog.Debug(ctx, "Using custom TLS client for Destroy() operation") - - destroyTlsConfig = &TlsConfig{ - CertFile: data.DestroyCertFile.ValueString(), - KeyFile: data.DestroyKeyFile.ValueString(), - CaCertFile: data.DestroyCaCertFile.ValueString(), - CaCertDirectory: data.DestroyCaCertDirectory.ValueString(), - SkipTlsVerify: data.DestroySkipTlsVerify.ValueBool(), - } + destroyTlsConfig = tlsConfigFromAttrs(data.DestroyCertFile, data.DestroyKeyFile, data.DestroyCaCertFile, data.DestroyCaCertDirectory, data.DestroySkipTlsVerify) } else { tflog.Debug(ctx, "Using default HTTP client for Destroy() operation") } diff --git a/internal/provider/curl_resource_test.go b/internal/provider/curl_resource_test.go index eeb6798..cdc225b 100644 --- a/internal/provider/curl_resource_test.go +++ b/internal/provider/curl_resource_test.go @@ -766,6 +766,48 @@ func TestAccCurlResourceWithTLSSkipVerify(t *testing.T) { }) } +func TestAccCurlResourceSkipTlsVerifyOnly(t *testing.T) { + t.Setenv("TF_ACC", "true") + t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") + + server, certFile, keyFile, err := createTLSServer() + if err != nil { + t.Fatalf("failed to create TLS test server: %v", err) + } + defer server.Close() + defer func(name string) { + _ = os.Remove(name) + }(certFile) + defer func(name string) { + _ = os.Remove(name) + }(keyFile) + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + ProtoV6ProviderFactories: testAccProtoV6ProviderFactories, + Steps: []resource.TestStep{ + { + Config: testAccresourceCurlSkipTlsVerifyOnly("skip_tls_only", server.URL), + Check: resource.TestCheckResourceAttr("terracurl_request.skip_tls_only", "response", `{"message":"TLS test successful"}`), + }, + }, + }) +} + +func testAccresourceCurlSkipTlsVerifyOnly(name, url string) string { + return fmt.Sprintf(` +resource "terracurl_request" "skip_tls_only" { + name = "%s" + url = "%s" + method = "GET" + response_codes = ["200"] + skip_tls_verify = true + skip_read = true + skip_destroy = true +} +`, name, url) +} + func TestAccresourceCurlCreateSanitizesResponse(t *testing.T) { t.Setenv("TF_ACC", "true") t.Setenv("USE_DEFAULT_CLIENT_FOR_TESTS", "true") diff --git a/internal/provider/utilities.go b/internal/provider/utilities.go index e968b91..f3e646c 100644 --- a/internal/provider/utilities.go +++ b/internal/provider/utilities.go @@ -143,6 +143,20 @@ type TlsConfig struct { SkipTlsVerify bool } +func needsTlsClient(certFile, keyFile, caCertFile, caCertDirectory types.String, skipTlsVerify types.Bool) bool { + return !certFile.IsNull() || !keyFile.IsNull() || !caCertFile.IsNull() || !caCertDirectory.IsNull() || skipTlsVerify.ValueBool() +} + +func tlsConfigFromAttrs(certFile, keyFile, caCertFile, caCertDirectory types.String, skipTlsVerify types.Bool) *TlsConfig { + return &TlsConfig{ + CertFile: certFile.ValueString(), + KeyFile: keyFile.ValueString(), + CaCertFile: caCertFile.ValueString(), + CaCertDirectory: caCertDirectory.ValueString(), + SkipTlsVerify: skipTlsVerify.ValueBool(), + } +} + // defaultTlsConfig returns a default TlsConfig instance. func defaultTlsConfig() *TlsConfig { return &TlsConfig{} diff --git a/internal/provider/utilities_test.go b/internal/provider/utilities_test.go index c44b4f2..778501f 100644 --- a/internal/provider/utilities_test.go +++ b/internal/provider/utilities_test.go @@ -140,3 +140,20 @@ func TestTlsClientRequests(t *testing.T) { t.Errorf("Expected body 'success', got '%s'", body) } } + +func TestNeedsTlsClient(t *testing.T) { + nullString := types.StringNull() + nullBool := types.BoolNull() + + if needsTlsClient(nullString, nullString, nullString, nullString, nullBool) { + t.Error("expected false when all TLS attributes are unset") + } + + if !needsTlsClient(nullString, nullString, nullString, nullString, types.BoolValue(true)) { + t.Error("expected true when skip_tls_verify is true without cert attributes") + } + + if !needsTlsClient(types.StringValue("/path/to/cert.pem"), nullString, nullString, nullString, nullBool) { + t.Error("expected true when cert_file is set") + } +}