Merge pull request #319 from dikhan/deprecate-plugin-version-config

[BugFix: Issue #318] Deprecate plugin_version property from OpenAPI config file SchemaV1

docs/plugin_configuration_schema.md

@@ -69,7 +69,6 @@ Describes the configurations available on a single service.
 Field Name | Type | Description
 ---|:---:|---
 swagger-url | `string` | **Required.** Defines the location where the swagger document is hosted. The value must be either a valid formatted URL or a path to a swagger file stored in the disk
-plugin_version | `string` | Defines the plugin version. If this value is specified (and it is not an empty string), the openapi plugin version executed must match this value; otherwise the validation will fail throwing an error at runtime. If the property is not set at all or the property is set with a value of empty string, then the default behaviour is that no validation will be performed.
 insecure_skip_verify | `string` | Defines whether a certificate verification should be performed when retrieving ```swagger-url``` from the server. This is **not recommended** for regular use and should only be set when the server hosting the swagger file is known and trusted but does not have a cert signed by the usually trusted CAs.
 schema_configuration | [][Schema Configuration Object](https://github.com/dikhan/terraform-provider-openapi/blob/master/docs/plugin_configuration_schema.md#schema-configuration-object) |  | Schema Configuration Object
 telemetry | [Telemetry Object](#telemetry-object) | Telemetry configuration

openapi/plugin_config.go

@@ -4,7 +4,6 @@ import (
 	"bufio"
 	"fmt"
 	"github.com/dikhan/terraform-provider-openapi/v2/openapi/terraformutils"
-	"github.com/dikhan/terraform-provider-openapi/v2/openapi/version"
 	"gopkg.in/yaml.v2"
 	"io"
 	"io/ioutil"
@@ -133,7 +132,7 @@ func (p *PluginConfiguration) getServiceConfiguration() (ServiceConfiguration, e
 		return nil, fmt.Errorf("swagger url not provided, please export OTF_VAR_<provider_name>_SWAGGER_URL env variable with the URL where '%s' service provider is exposing the swagger file OR create a plugin configuration file at ~/.terraform.d/plugins following the Plugin configuration schema specifications", p.ProviderName)
 	}
 
-	if err = serviceConfig.Validate(version.Version); err != nil {
+	if err = serviceConfig.Validate(); err != nil {
 		return nil, fmt.Errorf("service configuration for '%s' not valid: %s", p.ProviderName, err)
 	}
 

openapi/plugin_config_schema.go

@@ -28,7 +28,6 @@ type PluginConfigSchema interface {
 // services:
 //   monitor:
 //     swagger-url: http://monitor-api.com/swagger.json
-//     plugin_version: 0.14.0
 //     insecure_skip_verify: true
 //   cdn:
 //     swagger-url: https://cdn-api.com/swagger.json

openapi/plugin_config_schema_test.go

@@ -155,13 +155,11 @@ func TestPluginConfigSchemaV1Marshal(t *testing.T) {
 	Convey("Given a PluginConfigSchemaV1 containing a version supported and some services containing telemetry", t, func() {
 		var pluginConfigSchema PluginConfigSchema
 		expectedURL := "http://sevice-api.com/swagger.yaml"
-		expectedPluginVersion := "0.14.0"
 		serviceConfigName := "test"
 		expectedInscureSkipVerify := true
 		services := map[string]*ServiceConfigV1{
 			serviceConfigName: {
 				SwaggerURL:         expectedURL,
-				PluginVersion:      expectedPluginVersion,
 				InsecureSkipVerify: expectedInscureSkipVerify,
 				TelemetryConfig: &TelemetryConfig{
 					Graphite: &TelemetryProviderGraphite{
@@ -198,7 +196,6 @@ func TestPluginConfigSchemaV1Marshal(t *testing.T) {
 services:
   test:
     swagger-url: %s
-    plugin_version: %s
     insecure_skip_verify: %t
     schema_configuration:
     - schema_property_name: apikey_auth
@@ -217,7 +214,7 @@ services:
       http_endpoint:
         url: http://my-api.com/v1/metrics
         prefix: some_prefix
-`, expectedURL, expectedPluginVersion, expectedInscureSkipVerify)
+`, expectedURL, expectedInscureSkipVerify)
 				So(string(marshalConfig), ShouldEqual, expectedConfig)
 			})
 		})
@@ -231,7 +228,7 @@ services:
 		services := map[string]*ServiceConfigV1{
 			serviceConfigName: {
 				SwaggerURL: expectedURL,
-				//PluginVersion: expectedPluginVersion,
+				//PluginVersion: expectedPluginVersion, Note: This functionality has been deprecated as of OpenAPI version > 2.2.0. Nonetheless, this test confirms backwards compatibility with configurations that are still specifying the plugin_version property
 				InsecureSkipVerify: expectedInscureSkipVerify,
 				SchemaConfigurationV1: []ServiceSchemaPropertyConfigurationV1{
 					{
@@ -276,12 +273,11 @@ services:
 		var pluginConfigSchema PluginConfigSchema
 		expectedURL := "http://sevice-api.com/swagger.yaml"
 		serviceConfigName := "test"
-		expectedInscureSkipVerify := true
+		expectedInsecureSkipVerify := true
 		services := map[string]*ServiceConfigV1{
 			serviceConfigName: {
 				SwaggerURL:         expectedURL,
-				PluginVersion:      "0.14.0",
-				InsecureSkipVerify: expectedInscureSkipVerify,
+				InsecureSkipVerify: expectedInsecureSkipVerify,
 				SchemaConfigurationV1: []ServiceSchemaPropertyConfigurationV1{
 					{
 						SchemaPropertyName: "apikey_auth",
@@ -299,12 +295,11 @@ services:
 services:
   test:
     swagger-url: %s
-    plugin_version: 0.14.0
     insecure_skip_verify: %t
     schema_configuration:
     - schema_property_name: apikey_auth
       default_value: apiKeyValue
-`, expectedURL, expectedInscureSkipVerify)
+`, expectedURL, expectedInsecureSkipVerify)
 				So(string(marshalConfig), ShouldEqual, expectedConfig)
 			})
 		})

openapi/plugin_config_services.go

@@ -11,16 +11,13 @@ import (
 type ServiceConfiguration interface {
 	// GetSwaggerURL returns the URL where the service swagger doc is exposed
 	GetSwaggerURL() string
-	// GetSPluginVersion returns the OpenAPI Plugin version
-	GetPluginVersion() string
 	// IsInsecureSkipVerifyEnabled returns true if the given provider's service configuration has InsecureSkipVerify enabled; false
 	// otherwise
 	IsInsecureSkipVerifyEnabled() bool
 	// GetSchemaPropertyConfiguration returns the schema configuration for the given schemaPropertyName
 	GetSchemaPropertyConfiguration(schemaPropertyName string) ServiceSchemaPropertyConfiguration
 	// Validate makes sure the configuration is valid
-	Validate(runningPluginVersion string) error
-
+	Validate() error
 	// GetTelemetryConfiguration returns the telemetry configuration for this service provider
 	GetTelemetryConfiguration() TelemetryProvider
 }
@@ -37,8 +34,6 @@ type TelemetryConfig struct {
 type ServiceConfigV1 struct {
 	// SwaggerURL defines where the swagger is located
 	SwaggerURL string `yaml:"swagger-url"`
-	// PluginVersion defines the version of the OpenAPI Terraform plugin installed when generating the plugin configuration
-	PluginVersion string `yaml:"plugin_version,omitempty"`
 	// InsecureSkipVerify defines whether the internal http client used to fetch the swagger file should verify the server cert
 	// or not. This should only be used purposefully if the server is using a self-signed cert and only if the server is trusted
 	InsecureSkipVerify bool `yaml:"insecure_skip_verify,omitempty"`
@@ -63,11 +58,6 @@ func (s *ServiceConfigV1) GetSwaggerURL() string {
 	return s.SwaggerURL
 }
 
-// GetPluginVersion returns the OpenAPI Plugin version
-func (s *ServiceConfigV1) GetPluginVersion() string {
-	return s.PluginVersion
-}
-
 // IsInsecureSkipVerifyEnabled returns true if the given provider's service configuration has InsecureSkipVerify enabled; false
 // otherwise
 func (s *ServiceConfigV1) IsInsecureSkipVerifyEnabled() bool {
@@ -118,19 +108,12 @@ func (s *ServiceConfigV1) GetSchemaPropertyConfiguration(schemaPropertyName stri
 }
 
 // Validate makes sure the configuration is valid:
-// - if the user has specified an OpenAPI plugin version, and if the plugin does not match the version then something is off
-func (s *ServiceConfigV1) Validate(runningPluginVersion string) error {
+func (s *ServiceConfigV1) Validate() error {
 	if !govalidator.IsURL(s.SwaggerURL) {
 		// fall back to try to load the swagger file from disk in case the path provided is a path to a file on disk
 		if _, err := os.Stat(s.SwaggerURL); os.IsNotExist(err) {
 			return fmt.Errorf("service swagger URL configuration not valid ('%s'). URL must be either a valid formed URL or a path to an existing swagger file stored in the disk", s.SwaggerURL)
 		}
 	}
-	if s.PluginVersion != "" {
-		if s.PluginVersion != runningPluginVersion {
-			return fmt.Errorf("plugin version '%s' in the plugin configuration file does not match the version of the OpenAPI plugin that is running '%s'", s.PluginVersion, runningPluginVersion)
-		}
-	}
-
 	return nil
 }

openapi/plugin_config_services_stub.go

@@ -27,18 +27,13 @@ func (s *ServiceConfigStub) GetSwaggerURL() string {
 	return s.SwaggerURL
 }
 
-// GetPluginVersion returns the plugin version value configured in the ServiceConfigStub.PluginVersion field
-func (s *ServiceConfigStub) GetPluginVersion() string {
-	return s.PluginVersion
-}
-
 // IsInsecureSkipVerifyEnabled returns the bool configured in the ServiceConfigStub.InsecureSkipVerify field
 func (s *ServiceConfigStub) IsInsecureSkipVerifyEnabled() bool {
 	return s.InsecureSkipVerify
 }
 
 // Validate returns an error if the ServiceConfigStub.Err field is set with an error
-func (s *ServiceConfigStub) Validate(runningPluginVersion string) error {
+func (s *ServiceConfigStub) Validate() error {
 	return s.Err
 }
 

openapi/plugin_config_services_test.go

@@ -38,22 +38,6 @@ func TestServiceConfigV1GetSwaggerURL(t *testing.T) {
 	})
 }
 
-func TestServiceConfigV1GetPluginVersion(t *testing.T) {
-	Convey("Given a ServiceConfigV1 containing a specific plugin version", t, func() {
-		var serviceConfiguration ServiceConfiguration
-		expectedPluginVersion := "0.14.0"
-		serviceConfiguration = &ServiceConfigV1{
-			PluginVersion: expectedPluginVersion,
-		}
-		Convey("When GetPluginVersion method is called", func() {
-			pluginVersion := serviceConfiguration.GetPluginVersion()
-			Convey("Then the plugin version returned should be equal to expected one", func() {
-				So(pluginVersion, ShouldEqual, expectedPluginVersion)
-			})
-		})
-	})
-}
-
 func TestServiceConfigV1IsSecureSkipVerifyEnabled(t *testing.T) {
 	Convey("Given a ServiceConfigV1 containing the insecure_skip_verify enabled", t, func() {
 		var serviceConfiguration ServiceConfiguration
@@ -101,22 +85,6 @@ func TestGetSchemaPropertyConfiguration(t *testing.T) {
 }
 
 func TestServiceConfigV1Validate(t *testing.T) {
-	Convey("Given a ServiceConfigV1 containing a valid swagger URL and a specific plugin version", t, func() {
-		var serviceConfiguration ServiceConfiguration
-		expectedPluginVersion := "0.14.0"
-		expectedSwaggerURL := "http://sevice-api.com/swagger.yaml"
-		serviceConfiguration = &ServiceConfigV1{
-			SwaggerURL:    expectedSwaggerURL,
-			PluginVersion: expectedPluginVersion,
-		}
-		Convey("When Validate method is called with a running version that matches the configured one", func() {
-			runningPluginVersion := expectedPluginVersion
-			err := serviceConfiguration.Validate(runningPluginVersion)
-			Convey("Then the error returned should be nil", func() {
-				So(err, ShouldBeNil)
-			})
-		})
-	})
 	Convey("Given a ServiceConfigV1 containing an invalid swagger URL pointing at a file store in the disk", t, func() {
 		var serviceConfiguration ServiceConfiguration
 		swaggerFile, _ := ioutil.TempFile("", "")
@@ -129,20 +97,7 @@ func TestServiceConfigV1Validate(t *testing.T) {
 			SwaggerURL: expectedSwaggerURL,
 		}
 		Convey("When Validate method is called", func() {
-			err := serviceConfiguration.Validate("0.14.0")
-			Convey("Then the error returned should be nil", func() {
-				So(err, ShouldBeNil)
-			})
-		})
-	})
-	Convey("Given a ServiceConfigV1 containing an empty plugin version", t, func() {
-		expectedSwaggerURL := "http://a.valid.url"
-		serviceConfiguration := &ServiceConfigV1{
-			SwaggerURL:    expectedSwaggerURL,
-			PluginVersion: "",
-		}
-		Convey("When Validate method is called", func() {
-			err := serviceConfiguration.Validate("0.14.0")
+			err := serviceConfiguration.Validate()
 			Convey("Then the error returned should be nil", func() {
 				So(err, ShouldBeNil)
 			})
@@ -155,29 +110,12 @@ func TestServiceConfigV1Validate(t *testing.T) {
 			SwaggerURL: expectedSwaggerURL,
 		}
 		Convey("When Validate method is called", func() {
-			err := serviceConfiguration.Validate("0.14.0")
+			err := serviceConfiguration.Validate()
 			Convey("Then the error returned should be the expected one", func() {
 				So(err.Error(), ShouldEqual, "service swagger URL configuration not valid ('htpt:/non-valid-url'). URL must be either a valid formed URL or a path to an existing swagger file stored in the disk")
 			})
 		})
 	})
-
-	Convey("Given a ServiceConfigV1 containing a valid swagger file and a specific plugin version", t, func() {
-		var serviceConfiguration ServiceConfiguration
-		expectedPluginVersion := "0.14.0"
-		expectedSwaggerURL := "http://sevice-api.com/swagger.yaml"
-		serviceConfiguration = &ServiceConfigV1{
-			SwaggerURL:    expectedSwaggerURL,
-			PluginVersion: expectedPluginVersion,
-		}
-		Convey("When Validate method is called with a running version that DOES NOT match the configured one", func() {
-			runningPluginVersion := "0.15.0"
-			err := serviceConfiguration.Validate(runningPluginVersion)
-			Convey("Then the error returned should be the expected one", func() {
-				So(err.Error(), ShouldEqual, "plugin version '0.14.0' in the plugin configuration file does not match the version of the OpenAPI plugin that is running '0.15.0'")
-			})
-		})
-	})
 }
 
 func TestGetTelemetryConfiguration(t *testing.T) {

openapi/plugin_config_test.go

@@ -1,6 +1,7 @@
 package openapi
 
 import (
+	"errors"
 	"fmt"
 	"github.com/smartystreets/assertions/should"
 	. "github.com/smartystreets/goconvey/convey"
@@ -54,6 +55,14 @@ func TestGetPluginConfigurationPath(t *testing.T) {
 	})
 }
 
+type errReader struct {
+	errorMessage string
+}
+
+func (e errReader) Read(p []byte) (n int, err error) {
+	return 0, errors.New(e.errorMessage)
+}
+
 func TestGetServiceProviderConfiguration(t *testing.T) {
 	Convey("Given a PluginConfiguration for 'test' provider and a OTF_VAR_test_SWAGGER_URL is set using lower case provider name", t, func() {
 		pluginConfiguration, _ := NewPluginConfiguration(providerName)
@@ -256,12 +265,25 @@ services:
 		})
 	})
 
-	Convey("Given a PluginConfiguration for 'test' provider and a plugin configuration file containing a service configuration that is invalid (e,g: plugin version is specified but does not match the version of the plugin executing)", t, func() {
+	Convey("Given a PluginConfiguration that fails to read", t, func() {
+		errReader := errReader{"some error"}
+		pluginConfiguration := PluginConfiguration{
+			ProviderName:  providerName,
+			Configuration: errReader,
+		}
+		Convey("When getServiceConfiguration is called", func() {
+			_, err := pluginConfiguration.getServiceConfiguration()
+			Convey("Then the error should containing the following message", func() {
+				So(err.Error(), should.ContainSubstring, "failed to read terraform-provider-openapi.yaml configuration file")
+			})
+		})
+	})
+
+	Convey("Given a PluginConfiguration for 'test' provider and a plugin configuration file containing a service called 'test' with non valid configuration", t, func() {
 		pluginConfig := fmt.Sprintf(`version: '1'
 services:
-    %s:
-        swagger-url: %s
-        plugin_version: 0.14.0`, providerName, otfVarSwaggerURLValue)
+   %s:
+       swagger-url: non-valid-url`, providerName)
 		configReader := strings.NewReader(pluginConfig)
 		pluginConfiguration := PluginConfiguration{
 			ProviderName:  providerName,
@@ -270,7 +292,7 @@ services:
 		Convey("When getServiceConfiguration is called", func() {
 			_, err := pluginConfiguration.getServiceConfiguration()
 			Convey("Then the error should containing the following message", func() {
-				So(err.Error(), should.ContainSubstring, "service configuration for 'test' not valid: plugin version '0.14.0' in the plugin configuration file does not match the version of the OpenAPI plugin that is running 'dev'")
+				So(err.Error(), should.ContainSubstring, "service configuration for 'test' not valid: service swagger URL configuration not valid ('non-valid-url'). URL must be either a valid formed URL or a path to an existing swagger file stored in the disk")
 			})
 		})
 	})

openapi/terraformutils/terraform_utils.go

@@ -150,8 +150,7 @@ func multiEnvDefaultFunc(ks []string, defaultValue interface{}) schema.SchemaDef
 
 // MultiEnvDefaultString is a helper function that returns the value as string of the first
 // environment variable in the given list 'ks' that returns a non-empty value. If none of the environment variables
-// return a value, the default value is
-// returned.
+// return a value, the default value is returned.
 func MultiEnvDefaultString(ks []string, defaultValue interface{}) (string, error) {
 	dv, err := multiEnvDefaultFunc(ks, defaultValue)()
 	if err != nil {