From 75327df076cd3896ca4065ff2bcb2694fe67563a Mon Sep 17 00:00:00 2001 From: FSJ <7189895+ShiftedMr@users.noreply.github.com> Date: Mon, 3 Mar 2025 14:12:02 +0000 Subject: [PATCH] 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 | 28 +++- go.mod | 4 + go.sum | 4 + main.go | 6 + plugin/__test__/package.json | 8 + plugin/__testwithport__/package.json | 8 + plugin/impl.go | 64 ++++++-- plugin/impl_test.go | 229 ++++++++++++++++++++++++++- 8 files changed, 336 insertions(+), 15 deletions(-) create mode 100644 plugin/__test__/package.json create mode 100644 plugin/__testwithport__/package.json diff --git a/README.md b/README.md index 045cbb9..358ec8a 100644 --- a/README.md +++ b/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 +``` \ No newline at end of file diff --git a/go.mod b/go.mod index 039b83e..cdf9902 100644 --- a/go.mod +++ b/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 ) diff --git a/go.sum b/go.sum index 6ea65ff..4acf242 100644 --- a/go.sum +++ b/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= diff --git a/main.go b/main.go index efa57e9..926d264 100644 --- a/main.go +++ b/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, + }, } } diff --git a/plugin/__test__/package.json b/plugin/__test__/package.json new file mode 100644 index 0000000..35215ea --- /dev/null +++ b/plugin/__test__/package.json @@ -0,0 +1,8 @@ +{ + "name": "my-awesome-package", + "version": "1.0.0", + "author": "Your Name (https://example.com)", + "publishConfig": { + "registry": "https://fakenpm.reg.org/good/path" + } +} diff --git a/plugin/__testwithport__/package.json b/plugin/__testwithport__/package.json new file mode 100644 index 0000000..017ef18 --- /dev/null +++ b/plugin/__testwithport__/package.json @@ -0,0 +1,8 @@ +{ + "name": "my-awesome-package", + "version": "1.0.0", + "author": "Your Name (https://example.com)", + "publishConfig": { + "registry": "https://fakenpm.reg.org:443/good/path" + } +} diff --git a/plugin/impl.go b/plugin/impl.go index 5b11e0a..42557e1 100644 --- a/plugin/impl.go +++ b/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 } diff --git a/plugin/impl_test.go b/plugin/impl_test.go index badfaa0..f52be4d 100644 --- a/plugin/impl_test.go +++ b/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) {