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

@@ -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"; \

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
 )

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=

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)

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
 }

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) {