From c7a72eae0dc29ad4e284f3dee7e0567288292f51 Mon Sep 17 00:00:00 2001 From: Daniel Isaac Khan Ramiro Date: Tue, 8 Jun 2021 20:35:34 -0700 Subject: [PATCH] Issue #300: ServiceConfiguration with InsecureSkipVerify enabled now sets the InsecureSkipVerify in the tr.TLSClientConfig accordingly - Moved the logic that handled the TLSClient InsecureSkipVerify enablement from getServiceConfiguration to the CreateSchemaProviderFromServiceConfiguration. - Added more logging when the InsecureSkipVerify is enabled with appropriate warnings - Added more test coverage on CreateSchemaProviderFromServiceConfiguration method - Moved the skipVerify env variable check if the otfVarSwaggerURL is also set as it was only used there but yet initialized outside for no particular reason. --- Makefile | 2 +- go.mod | 2 +- go.sum | 2 + openapi/plugin_config.go | 4 +- openapi/provider.go | 23 ++++---- openapi/provider_test.go | 124 ++++++++++++++++++++++++++++++++++++--- 6 files changed, 132 insertions(+), 25 deletions(-) diff --git a/Makefile b/Makefile index d689d9f30..a8e2482dd 100644 --- a/Makefile +++ b/Makefile @@ -80,7 +80,7 @@ ifdef PERFORM_DOCKER_LOGIN endif # make integration-test -integration-test: local-env-down local-env show-terraform-version dockerhub-login +integration-test: dockerhub-login local-env-down local-env show-terraform-version @echo "[INFO] Executing integration tests for $(TF_OPENAPI_PROVIDER_PLUGIN_NAME)" @TF_ACC=true go test -v -cover $(INT_TEST_PACKAGES) ; if [ $$? -eq 1 ]; then \ echo "[ERROR] Test returned with failures. Please go through the different scenarios and fix the tests that are failing"; \ diff --git a/go.mod b/go.mod index 3bb471da6..9cc76508d 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,6 @@ require ( github.com/stretchr/testify v1.6.1 golang.org/x/crypto v0.0.0-20200820211705-5c72a883971a // indirect golang.org/x/lint v0.0.0-20210508222113-6edffad5e616 // indirect - golang.org/x/tools v0.1.1 // indirect + golang.org/x/tools v0.1.2 // indirect gopkg.in/yaml.v2 v2.2.4 ) diff --git a/go.sum b/go.sum index 63390a43e..29779463d 100644 --- a/go.sum +++ b/go.sum @@ -583,6 +583,8 @@ golang.org/x/tools v0.0.0-20200713011307-fd294ab11aed/go.mod h1:njjCfa9FT2d7l9Bc golang.org/x/tools v0.0.0-20200717024301-6ddee64345a6/go.mod h1:njjCfa9FT2d7l9Bc6FUM5FLjQPp3cFF28FI3qnDFljA= golang.org/x/tools v0.1.1 h1:wGiQel/hW0NnEkJUk8lbzkX2gFJU6PFxf1v5OlCfuOs= golang.org/x/tools v0.1.1/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= +golang.org/x/tools v0.1.2 h1:kRBLX7v7Af8W7Gdbbc908OJcdgtK8bOz9Uaj8/F1ACA= +golang.org/x/tools v0.1.2/go.mod h1:o0xws9oXOQQZyjljx8fwUC0k7L1pTE6eaCbjGeHmOkk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/openapi/plugin_config.go b/openapi/plugin_config.go index adb82cdfe..b701739d3 100644 --- a/openapi/plugin_config.go +++ b/openapi/plugin_config.go @@ -86,8 +86,6 @@ func (p *PluginConfiguration) getServiceConfiguration() (ServiceConfiguration, e var serviceConfig ServiceConfiguration var err error - skipVerify, _ := strconv.ParseBool(os.Getenv(otfVarInsecureSkipVerify)) - swaggerURLEnvVar := fmt.Sprintf(otfVarSwaggerURL, p.ProviderName) swaggerURLEnvVars := []string{swaggerURLEnvVar, strings.ToUpper(swaggerURLEnvVar)} apiDiscoveryURL, err := terraformutils.MultiEnvDefaultString(swaggerURLEnvVars, "") @@ -97,6 +95,8 @@ func (p *PluginConfiguration) getServiceConfiguration() (ServiceConfiguration, e // Found OTF_VAR_%s_SWAGGER_URL env variable if apiDiscoveryURL != "" { log.Printf("[INFO] %s set with value %s", swaggerURLEnvVar, apiDiscoveryURL) + skipVerify, _ := strconv.ParseBool(os.Getenv(otfVarInsecureSkipVerify)) + log.Printf("[INFO] %s set with value %t", otfVarInsecureSkipVerify, skipVerify) pluginConfigV1.Services = map[string]*ServiceConfigV1{} pluginConfigV1.Services[p.ProviderName] = NewServiceConfigV1(apiDiscoveryURL, skipVerify, nil) serviceConfig, err = pluginConfigV1.GetServiceConfig(p.ProviderName) diff --git a/openapi/provider.go b/openapi/provider.go index ba45d7d35..1f5978d56 100644 --- a/openapi/provider.go +++ b/openapi/provider.go @@ -1,12 +1,10 @@ package openapi import ( - "net/http" - "crypto/tls" - "fmt" "log" + "net/http" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) @@ -38,6 +36,16 @@ func (p *ProviderOpenAPI) CreateSchemaProviderFromServiceConfiguration(serviceCo log.Printf("[DEBUG] service configuration = %+v", serviceConfiguration) + if serviceConfiguration.IsInsecureSkipVerifyEnabled() { + log.Printf("[WARN] Provider '%s' is using insecure skip verify, therefore the HTTPs client will not verify the API server's certificate chain and host name. This should only be used for testing purposes and it's highly recommended avoiding the use of OTF_INSECURE_SKIP_VERIFY env variable or configuring the ServiceConfiguration with InsecureSkipVerifyEnabled when executing this provider", p.ProviderName) + tr := http.DefaultTransport.(*http.Transport) + // #nosec G402 + tr.TLSClientConfig = &tls.Config{ + InsecureSkipVerify: true, + } + log.Printf("[WARN] TLSClientConfig has been configured with InsecureSkipVerify set to true, this means that TLS connections will accept any certificate presented by the server and any host name in that certificate") + } + openAPISpecAnalyser, err := CreateSpecAnalyser(specAnalyserV2, serviceConfiguration.GetSwaggerURL()) if err != nil { return nil, fmt.Errorf("plugin OpenAPI spec analyser error: %s", err) @@ -69,15 +77,6 @@ func getServiceConfiguration(providerName string) (ServiceConfiguration, error) return nil, err } - if serviceConfiguration.IsInsecureSkipVerifyEnabled() { - tr := http.DefaultTransport.(*http.Transport) - // #nosec G402 - tr.TLSClientConfig = &tls.Config{ - InsecureSkipVerify: true, - } - log.Printf("[WARN] Provider '%s' is using insecure skip verify. Please make sure you trust the aforementioned server hosting the swagger file. Otherwise, it's highly recommended avoiding the use of OTF_INSECURE_SKIP_VERIFY env variable when executing this provider", providerName) - } - log.Printf("[INFO] Provider %s is using the following swagger file: %s", providerName, serviceConfiguration.GetSwaggerURL()) return serviceConfiguration, nil } diff --git a/openapi/provider_test.go b/openapi/provider_test.go index 7e24d2031..205ba7174 100644 --- a/openapi/provider_test.go +++ b/openapi/provider_test.go @@ -1,6 +1,7 @@ package openapi import ( + "errors" "fmt" "log" "net/http" @@ -191,11 +192,7 @@ definitions: providerName := "openapi" p := ProviderOpenAPI{ProviderName: providerName} tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{SwaggerURL: swaggerServer.URL}) - - Convey("Then ", func() { - - }) - Convey("And the provider returned should be configured as expected and the the error should be nil", func() { + Convey("Then the provider returned should be configured as expected and the the error should be nil", func() { So(err, ShouldBeNil) So(tfProvider, ShouldNotBeNil) @@ -530,10 +527,7 @@ definitions: providerName := "openapi" p := ProviderOpenAPI{ProviderName: providerName} tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{SwaggerURL: swaggerServer.URL}) - Convey("Then ", func() { - - }) - Convey("And the provider returned should be configured as expected and the error should be nil", func() { + Convey("Then the provider returned should be configured as expected and the error should be nil", func() { So(tfProvider, ShouldNotBeNil) So(err, ShouldBeNil) @@ -570,6 +564,118 @@ definitions: }) }) + Convey("Given a ProviderOpenAPI with an err field populated (this only happens once the provider has been initialized and the resulted provider/error have been cached in the ProviderOpenAPI", t, func() { + p := ProviderOpenAPI{ProviderName: providerName, err: errors.New("some error")} + Convey("When CreateSchemaProviderWithConfiguration method is called", func() { + tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{}) + Convey("Then the error returned should be the expecting one", func() { + So(err.Error(), ShouldEqual, "some error") + }) + Convey("And the tfProvider returned should be nil ", func() { + So(tfProvider, ShouldBeNil) + }) + }) + }) + + Convey("Given a ProviderOpenAPI with an provider field populated (this only happens once the provider has been initialized and the resulted provider/error have been cached in the ProviderOpenAPI", t, func() { + cachedProvider := &schema.Provider{} + p := ProviderOpenAPI{ProviderName: providerName, provider: cachedProvider} + Convey("When CreateSchemaProviderWithConfiguration method is called", func() { + tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{}) + Convey("Then the error returned should be the nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the tfProvider returned should be the cached one ", func() { + So(tfProvider, ShouldEqual, cachedProvider) + }) + }) + }) + + Convey("Given a ProviderOpenAPI", t, func() { + swaggerContent := `swagger: "2.0" +host: "localhost:8443" +basePath: "/api" +schemes: +- "https" +paths: + /v1/cdns: + x-terraform-resource-name: "cdn" + post: + summary: "Create cdn" + + parameters: + - in: "body" + name: "body" + description: "Created CDN" + required: true + schema: + $ref: "#/definitions/ContentDeliveryNetworkV1" + responses: + 201: + schema: + $ref: "#/definitions/ContentDeliveryNetworkV1" + /v1/cdns/{id}: + get: + summary: "Get cdn by id" + parameters: + - name: "id" + in: "path" + description: "The cdn id that needs to be fetched." + required: true + type: "string" + responses: + 200: + schema: + $ref: "#/definitions/ContentDeliveryNetworkV1" +definitions: + ContentDeliveryNetworkV1: + type: "object" + required: + - label + properties: + id: + type: "string" + readOnly: true + label: + type: "string"` + + swaggerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(swaggerContent)) + })) + p := ProviderOpenAPI{ProviderName: providerName} + Convey("When CreateSchemaProviderWithConfiguration method is called with a serviceConfiguration that has InsecureSkipVerifyEnabled set to true", func() { + tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{SwaggerURL: swaggerServer.URL, InsecureSkipVerify: true}) + Convey("Then the error returned should be the nil", func() { + So(err, ShouldBeNil) + }) + Convey("And the tfProvider returned should not be nil (skipping schema verification since that's covered in other tests)", func() { + So(tfProvider, ShouldNotBeNil) + }) + Convey("And default TLS transport configuration should be configured to skip verify", func() { + tr := http.DefaultTransport.(*http.Transport) + So(tr.TLSClientConfig.InsecureSkipVerify, ShouldBeTrue) + }) + + }) + }) + + Convey("Given a ProviderOpenAPI missing the providerName", t, func() { + swaggerContent := `swagger: "2.0"` + swaggerServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(swaggerContent)) + })) + p := ProviderOpenAPI{} + Convey("When CreateSchemaProviderWithConfiguration method is called", func() { + tfProvider, err := p.CreateSchemaProviderFromServiceConfiguration(&ServiceConfigStub{SwaggerURL: swaggerServer.URL}) + Convey("Then the error returned should be the nil", func() { + So(err.Error(), ShouldEqual, "plugin provider factory init error: provider name not specified") + }) + Convey("And the tfProvider returned should be nil", func() { + So(tfProvider, ShouldBeNil) + }) + }) + }) + } func Test_colliding_resource_names(t *testing.T) {