Updating validation to ignore default ports as well as a flag for skipping the registry validation. (#68)

* initial attempt at adding in default port agnostic configs for validation with a boolean to disable

* first run at tests

* more tests and fixing an error in logic in the impl file

* adding in extra package.jsons for testing

* adding in more tests to cover some more in depth possible variations

* Changing to a cleaner code and logic for the comparisons

* typo fix and a cleanup of the isNilOrStandardSchemePort logic

* making the new env var and settings name be consistent with the cli flag; formatting

* Adding in a README section for the new env var; as well as an example without said env var

README.md

@@ -35,7 +35,7 @@ docker build \
 ```
 
 ## Usage
-
+### Standard Local Usage
 ```console
 docker run --rm \
   -e NPM_USERNAME=drone \
@@ -45,3 +45,29 @@ docker run --rm \
   -w $(pwd) \
   plugins/npm
 ```
+#### With a specified registry for validation
+This will allow the setting of the defautl publishing registry. This will also raise a validation error if the publish configuration of the npm package is not pointing to the specified registry.
+```console
+docker run --rm \
+  -e NPM_USERNAME=drone \
+  -e NPM_PASSWORD=password \
+  -e NPM_EMAIL=drone@drone.io \
+  -e NPM_REGISTRY="https://fakenpm.reg.org/good/path" \
+  -v $(pwd):$(pwd) \
+  -w $(pwd) \
+  plugins/npm
+```
+
+#### Ignore registry validation
+This will all the setting of a default publishing registry but will skip the verification of it being the same as the one in the npmrc. In this instance no validation error is raised and the registry in the npm rc is used
+```console
+docker run --rm \
+  -e NPM_USERNAME=drone \
+  -e NPM_PASSWORD=password \
+  -e NPM_EMAIL=drone@drone.io \
+  -e NPM_REGISTRY="https://fakenpm.reg.org/good/path" \
+  -e PLUGIN_SKIP_REGISTRY_VALIDATION=true \
+  -v $(pwd):$(pwd) \
+  -w $(pwd) \
+  plugins/npm
+```

go.mod

@@ -11,7 +11,11 @@ require (
 
 require (
 	github.com/cpuguy83/go-md2man/v2 v2.0.2 // indirect
+	github.com/davecgh/go-spew v1.1.1 // indirect
+	github.com/pmezard/go-difflib v1.0.0 // indirect
 	github.com/russross/blackfriday/v2 v2.1.0 // indirect
+	github.com/stretchr/testify v1.10.0 // indirect
 	github.com/xrash/smetrics v0.0.0-20201216005158-039620a65673 // indirect
 	golang.org/x/sys v0.3.0 // indirect
+	gopkg.in/yaml.v3 v3.0.1 // indirect
 )

go.sum

@@ -30,6 +30,8 @@ github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+
 github.com/stretchr/testify v1.2.2 h1:bSDNvY7ZPG5RlJ8otE/7V6gMiyenm9RtJ7IUVIAoJ1w=
 github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
 github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA=
+github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 github.com/urfave/cli/v2 v2.2.0/go.mod h1:SE9GqnLQmjVa0iPEY0f1w3ygNIYcIJ0OKPMoW2caLfQ=
 github.com/urfave/cli/v2 v2.8.1 h1:CGuYNZF9IKZY/rfBe3lJpccSoIY1ytfvmgQT90cNOl4=
 github.com/urfave/cli/v2 v2.8.1/go.mod h1:Z41J9TPoffeoqP0Iza0YbAhGvymRdZAd2uPmZ5JxRdY=
@@ -52,4 +54,6 @@ golang.org/x/tools v0.0.0-20190624222133-a101b041ded4/go.mod h1:/rFqwRUd4F7ZHNgw
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
 gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
+gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
+gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gotest.tools/v3 v3.0.2/go.mod h1:3SzNCllyD9/Y+b5r9JIKQ474KzkZyqLqEfYqMsX94Bk=

main.go

@@ -132,5 +132,11 @@ func settingsFlags(settings *plugin.Settings) []cli.Flag {
 			EnvVars:     []string{"PLUGIN_ACCESS"},
 			Destination: &settings.Access,
 		},
+		&cli.BoolFlag{
+			Name:        "skip-registry-validation",
+			Usage:       "skips validation for uri in package.json and the currently configured registry",
+			EnvVars:     []string{"PLUGIN_SKIP_REGISTRY_VALIDATION"},
+			Destination: &settings.SkipRegistryValidation,
+		},
 	}
 }

plugin/__test__/package.json

@@ -0,0 +1,8 @@
+{
+    "name": "my-awesome-package",
+    "version": "1.0.0",
+    "author": "Your Name <email@example.com> (https://example.com)",
+    "publishConfig": {
+        "registry": "https://fakenpm.reg.org/good/path"
+    }
+}

plugin/__testwithport__/package.json

@@ -0,0 +1,8 @@
+{
+    "name": "my-awesome-package",
+    "version": "1.0.0",
+    "author": "Your Name <email@example.com> (https://example.com)",
+    "publishConfig": {
+        "registry": "https://fakenpm.reg.org:443/good/path"
+    }
+}

plugin/impl.go

@@ -23,16 +23,17 @@ import (
 type (
 	// Settings for the Plugin.
 	Settings struct {
-		Username              string
-		Password              string
-		Token                 string
-		SkipWhoami            bool
-		Email                 string
-		Registry              string
-		Folder                string
-		FailOnVersionConflict bool
-		Tag                   string
-		Access                string
+		Username               string
+		Password               string
+		Token                  string
+		SkipWhoami             bool
+		Email                  string
+		Registry               string
+		Folder                 string
+		FailOnVersionConflict  bool
+		Tag                    string
+		Access                 string
+		SkipRegistryValidation bool
 
 		npm *npmPackage
 	}
@@ -51,6 +52,42 @@ type (
 // globalRegistry defines the default NPM registry.
 const globalRegistry = "https://registry.npmjs.org/"
 
+// May be better as an enum in order to make it a const
+var defaultPortMap = map[string]string{
+	"http":  "80",
+	"https": "443",
+}
+
+func isNilPortOrStandardSchemePort(u *url.URL) bool {
+	if u.Scheme != "http" && u.Scheme != "https" {
+		//invalid schemes aren't worth checking and we want http or https
+		return false
+	}
+	// since we verify above that the scheme above is valid and this map
+	// is initialized in this file. It's safe to assume the key is in the map
+	return u.Port() == "" || u.Port() == defaultPortMap[u.Scheme]
+}
+
+func (p *Plugin) CompareRegistries(nc npmConfig) (bool, error) {
+	parsedConfigReg, err := url.Parse(nc.Registry)
+	if err != nil {
+		return false, fmt.Errorf("package.json registry: %s failed to parse", nc.Registry)
+	}
+	parsedSettingsReg, err := url.Parse(p.settings.Registry)
+	if err != nil {
+		return false, fmt.Errorf("drone yaml npm Registry: %s failed to parse", p.settings.Registry)
+	}
+
+	ncDefaultOrNilPort := isNilPortOrStandardSchemePort(parsedConfigReg)
+	dyDefaultOrNilPort := isNilPortOrStandardSchemePort(parsedSettingsReg)
+
+	matchingStatus := parsedSettingsReg.Scheme == parsedConfigReg.Scheme &&
+		parsedSettingsReg.Path == parsedConfigReg.Path &&
+		parsedSettingsReg.Hostname() == parsedConfigReg.Hostname() &&
+		dyDefaultOrNilPort == ncDefaultOrNilPort
+	return matchingStatus, nil
+}
+
 // Validate handles the settings validation of the plugin.
 func (p *Plugin) Validate() error {
 	// Check authentication options
@@ -84,12 +121,15 @@ func (p *Plugin) Validate() error {
 		p.settings.Registry = globalRegistry
 	}
 
-	if strings.Compare(p.settings.Registry, npm.Config.Registry) != 0 {
+	registriesMatch, err := p.CompareRegistries(npm.Config)
+	if err != nil {
+		return fmt.Errorf("issue comparing the registries specified in drone yaml (%s) and package.json: (%s)", p.settings.Registry, npm.Config.Registry) // if there's an error using this default to standard validation by string compare
+	}
+	if !registriesMatch && !p.settings.SkipRegistryValidation {
 		return fmt.Errorf("registry values do not match .drone.yml: %s package.json: %s", p.settings.Registry, npm.Config.Registry)
 	}
 
 	p.settings.npm = npm
-
 	return nil
 }
 

plugin/impl_test.go

@@ -6,11 +6,236 @@
 package plugin
 
 import (
+	"context"
+	"net/url"
 	"testing"
+
+	"github.com/drone-plugins/drone-plugin-lib/drone"
+	"github.com/stretchr/testify/assert"
 )
 
-func TestValidate(t *testing.T) {
-	t.Skip()
+func initFakeSettings() Settings {
+	nc := npmConfig{
+		// Note: this registry is the one that would come from publishConfig in package.json
+		Registry: "SetByFn_readPackageFile",
+	}
+	np := npmPackage{
+		Name:    "Test Package",
+		Version: "1.33.7",
+		Config:  nc,
+	}
+	return Settings{
+		Username:   "fakeUser",
+		Password:   "fakePass",
+		Token:      "",
+		SkipWhoami: false,
+		Email:      "fake@user.tst",
+		// Note: this registry is the one that would come from drone yaml
+		Registry:               "https://fakenpm.reg.org/good/path",
+		Folder:                 "__test__",
+		FailOnVersionConflict:  true,
+		Tag:                    "",
+		Access:                 "",
+		SkipRegistryValidation: false,
+		npm:                    &np,
+	}
+}
+
+func initFakeNetwork() drone.Network {
+	return drone.Network{
+		SkipVerify: true,
+		Client:     nil,
+		Context:    context.TODO(),
+	}
+}
+
+func initFakePipeline() drone.Pipeline {
+	return drone.Pipeline{
+		Build:  drone.Build{},
+		Repo:   drone.Repo{},
+		Commit: drone.Commit{},
+		Stage:  drone.Stage{},
+		Step:   drone.Step{},
+		SemVer: drone.SemVer{},
+		CalVer: drone.CalVer{},
+		System: drone.System{},
+	}
+}
+
+func initPlugin() *Plugin {
+	return &Plugin{
+		settings: initFakeSettings(),
+		pipeline: initFakePipeline(),
+		network:  initFakeNetwork(),
+	}
+}
+
+func getParsedUri(s string) *url.URL {
+	rslt, _ := url.Parse(s)
+	return rslt
+}
+
+func TestIsDefaultOrNilPort(t *testing.T) {
+	p := initPlugin()
+
+	resultWithoutPort := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	assert.Equal(t, true, resultWithoutPort)
+
+	p.settings.Registry = "https://fakenpm.reg.org:443"
+	resultWithPort := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	assert.Equal(t, true, resultWithPort)
+
+	p.settings.Registry = "http://fakenpm.reg.org:80"
+	resultWithPortHTTP := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	assert.Equal(t, true, resultWithPortHTTP)
+
+	p.settings.Registry = "fakenpm.reg.org"
+	resultWithoutSchemeOrPort := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	// npm requires scheme to be part of the url; so this function will return false for any missing a scheme
+	assert.Equal(t, false, resultWithoutSchemeOrPort)
+
+	p.settings.Registry = "fakenpm.reg.org:80"
+	resultWithoutScheme := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	assert.Equal(t, false, resultWithoutScheme)
+
+	p.settings.Registry = "https://fakenpm.reg.org:8443"
+	resultWithNonStandardPort := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	assert.Equal(t, false, resultWithNonStandardPort)
+
+	p.settings.Registry = "https://fakenpm.reg.org:8080"
+	resultWithNonStandardPortHTTP := isNilPortOrStandardSchemePort(getParsedUri(p.settings.Registry))
+	assert.Equal(t, false, resultWithNonStandardPortHTTP)
+}
+
+func TestCompareRegistries(t *testing.T) {
+	p := initPlugin()
+	goodReg := "https://fakenpm.reg.org/good/path"
+	goodRegWithPort := "https://fakenpm.reg.org:443/good/path"
+	goodRegWithNonStandardPort := "https://fakenpm.reg.org:8443/good/path"
+
+	p.settings.Registry = goodReg
+	p.settings.npm.Config.Registry = goodReg
+	ValidNoPorts, _ := p.CompareRegistries(p.settings.npm.Config)
+	assert.Equal(t, true, ValidNoPorts)
+
+	p.settings.Registry = goodRegWithPort
+	SameUrlOneWithPort, _ := p.CompareRegistries(p.settings.npm.Config)
+	assert.Equal(t, true, SameUrlOneWithPort)
+
+	p.settings.Registry = goodRegWithPort
+	p.settings.npm.Config.Registry = goodRegWithPort
+	SameUrlBothWithPort, _ := p.CompareRegistries(p.settings.npm.Config)
+	assert.Equal(t, true, SameUrlBothWithPort)
+
+	p.settings.Registry = "invalidUri"
+	invalidUriTest, _ := p.CompareRegistries(p.settings.npm.Config)
+	assert.Equal(t, false, invalidUriTest)
+
+	p.settings.Registry = goodRegWithNonStandardPort
+	nonStandardPortTest, _ := p.CompareRegistries(p.settings.npm.Config)
+	assert.Equal(t, false, nonStandardPortTest)
+}
+
+func TestValidateWithInvalidFields(t *testing.T) {
+	p := initPlugin()
+	// Validation tests with fields missing
+	p.settings.Email = ""
+	noEmailErr := p.Validate()
+	if assert.NotNil(t, noEmailErr) {
+		assert.Contains(t, noEmailErr.Error(), "email")
+	}
+
+	p.settings.Email = "fakeemail"
+	p.settings.Username = ""
+	noUserErr := p.Validate()
+	if assert.NotNil(t, noUserErr) {
+		assert.Contains(t, noUserErr.Error(), "username")
+	}
+
+	p.settings.Username = "fakeuser"
+	p.settings.Password = ""
+	noPassErr := p.Validate()
+	if assert.NotNil(t, noPassErr) {
+		assert.Contains(t, noPassErr.Error(), "password")
+	}
+
+	p.settings.Token = "fakeToken"
+	p.settings.Password = ""
+	p.settings.Username = ""
+	p.settings.Email = ""
+	tokenErr := p.Validate()
+	assert.Nil(t, tokenErr)
+}
+
+func TestValidateWithRegistryVariations(t *testing.T) {
+	p := initPlugin()
+
+	// Validation Tests with Invalid Registry
+	p.settings.Registry = "fakenpm.reg.org/good/path"
+	missingSchemeErr := p.Validate()
+	if assert.NotNil(t, missingSchemeErr) {
+		assert.Contains(t, missingSchemeErr.Error(), "fakenpm.reg.org")
+	}
+
+	p.settings.Registry = "https://fakenpm.reg.org:7894/good/path"
+	weirdPortErr := p.Validate()
+	if assert.NotNil(t, weirdPortErr) {
+		assert.Contains(t, weirdPortErr.Error(), "7894")
+	}
+
+	// Validation Tests with Default/NoPorts defined
+	p.settings.Registry = "https://fakenpm.reg.org:443/good/path"
+	defaultPortErr := p.Validate()
+	assert.Nil(t, defaultPortErr)
+
+	// Validation Tests with Failure Conditions on Registry
+
+	p.settings.Registry = "https://registry.npmjs.org/good/path"
+	diffRegistry := p.Validate()
+	if assert.NotNil(t, diffRegistry) {
+		assert.Contains(t, diffRegistry.Error(), "npmjs.org")
+	}
+
+	p.settings.Registry = "https://registry.npmjs.org:443/good/path"
+	diffRegistryWithPort := p.Validate()
+	if assert.NotNil(t, diffRegistryWithPort) {
+		assert.Contains(t, diffRegistryWithPort.Error(), "npmjs.org:443")
+	}
+
+	// Validation Failures with standardPorts But DiffPaths
+	p.settings.Registry = "https://registry.npmjs.org:443/bad/path"
+	p.settings.Folder = "__testwithport__"
+	diffRegistryWithPort = p.Validate()
+	if assert.NotNil(t, diffRegistryWithPort) {
+		assert.Contains(t, diffRegistryWithPort.Error(), "npmjs.org:443/bad")
+	}
+
+	// Same path different ports
+	p.settings.Registry = "https://registry.npmjs.org:8443/good/path"
+	p.settings.Folder = "__testwithport__"
+	diffRegistryWithPort = p.Validate()
+	if assert.NotNil(t, diffRegistryWithPort) {
+		assert.Contains(t, diffRegistryWithPort.Error(), "npmjs.org:8443/good")
+	}
+
+	// Same path different ports and schemes
+	p.settings.Registry = "http://registry.npmjs.org:80/good/path"
+	p.settings.Folder = "__testwithport__"
+	diffRegistryWithPort = p.Validate()
+	if assert.NotNil(t, diffRegistryWithPort) {
+		assert.Contains(t, diffRegistryWithPort.Error(), "npmjs.org:80/good")
+	}
+
+	// Validation Tests with SkipRegistryCheck
+	p.settings.SkipRegistryValidation = true
+	p.settings.Registry = "fakenpm.reg.org/good/path"
+	skipMissingSchemeErr := p.Validate()
+	assert.Nil(t, skipMissingSchemeErr)
+
+	p.settings.SkipRegistryValidation = true
+	p.settings.Registry = "https://fakenpm.reg.org:7894/good/path"
+	skipWeirdPortErr := p.Validate()
+	assert.Nil(t, skipWeirdPortErr)
 }
 
 func TestExecute(t *testing.T) {