diff --git a/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig.go b/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig.go index 42fc4e0a34..3d31f7c5cb 100644 --- a/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig.go +++ b/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig.go @@ -46,7 +46,7 @@ var ( type Config struct { // Name is the name of the plugin (e.g. 'buf.build/protocolbuffers/go'). Name bufpluginref.PluginIdentity - // Options is the set of options available to the plugin. + // Options is the default set of options passed into the plugin. // // For now, all options are string values. This could eventually // support other types (like JSON Schema and Terraform variables), @@ -73,26 +73,31 @@ type Config struct { // // Only one field will be set. type RuntimeConfig struct { - Go *GoRuntimeConfig - NPM *NPMRuntimeConfig - Archive *ArchiveRuntimeConfig + Go *GoRuntimeConfig + NPM *NPMRuntimeConfig } // GoRuntimeConfig is the runtime configuration for a Go plugin. type GoRuntimeConfig struct { MinVersion string - Deps []string + Deps []*GoRuntimeDependencyConfig +} + +// GoRuntimeDependencyConfig is the go runtime dependency configuration. +type GoRuntimeDependencyConfig struct { + Module string + Version string } // NPMRuntimeConfig is the runtime configuration for a JavaScript NPM plugin. type NPMRuntimeConfig struct { - Deps []string + Deps []*NPMRuntimeDependencyConfig } -// ArchiveRuntimeConfig is the runtime configuration for a plugin that can be downloaded -// as an archive instead of a language-specific registry. -type ArchiveRuntimeConfig struct { - Deps []string +// NPMRuntimeDependencyConfig is the npm runtime dependency configuration. +type NPMRuntimeDependencyConfig struct { + Package string + Version string } // GetConfigForBucket gets the Config for the YAML data at ConfigFilePath. @@ -162,16 +167,18 @@ type ExternalConfig struct { // ExternalRuntimeConfig is the external configuration for the runtime // of a plugin. type ExternalRuntimeConfig struct { - Go ExternalGoRuntimeConfig `json:"go,omitempty" yaml:"go,omitempty"` - NPM ExternalNPMRuntimeConfig `json:"npm,omitempty" yaml:"npm,omitempty"` - Archive ExternalArchiveRuntimeConfig `json:"archive,omitempty" yaml:"archive,omitempty"` + Go ExternalGoRuntimeConfig `json:"go,omitempty" yaml:"go,omitempty"` + NPM ExternalNPMRuntimeConfig `json:"npm,omitempty" yaml:"npm,omitempty"` } // ExternalGoRuntimeConfig is the external runtime configuration for a Go plugin. type ExternalGoRuntimeConfig struct { // The minimum Go version required by the plugin. - MinVersion string `json:"min_version,omitempty" yaml:"min_version,omitempty"` - Deps []string `json:"deps,omitempty" yaml:"deps,omitempty"` + MinVersion string `json:"min_version,omitempty" yaml:"min_version,omitempty"` + Deps []struct { + Module string `json:"module,omitempty" yaml:"module,omitempty"` + Version string `json:"version,omitempty" yaml:"version,omitempty"` + } `json:"deps,omitempty" yaml:"deps,omitempty"` } // IsEmpty returns true if the configuration is empty. @@ -181,7 +188,10 @@ func (e ExternalGoRuntimeConfig) IsEmpty() bool { // ExternalNPMRuntimeConfig is the external runtime configuration for a JavaScript NPM plugin. type ExternalNPMRuntimeConfig struct { - Deps []string `json:"deps,omitempty" yaml:"deps,omitempty"` + Deps []struct { + Package string `json:"package,omitempty" yaml:"package,omitempty"` + Version string `json:"version,omitempty" yaml:"version,omitempty"` + } `json:"deps,omitempty" yaml:"deps,omitempty"` } // IsEmpty returns true if the configuration is empty. @@ -189,17 +199,6 @@ func (e ExternalNPMRuntimeConfig) IsEmpty() bool { return len(e.Deps) == 0 } -// ExternalArchiveRuntimeConfig is the external runtime configuration for a plugin that can be -// downloaded as an archive instead of a language-specific registry. -type ExternalArchiveRuntimeConfig struct { - Deps []string `json:"deps,omitempty" yaml:"deps,omitempty"` -} - -// IsEmpty returns true if the configuration is empty. -func (e ExternalArchiveRuntimeConfig) IsEmpty() bool { - return len(e.Deps) == 0 -} - type externalConfigVersion struct { Version string `json:"version,omitempty" yaml:"version,omitempty"` } diff --git a/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig_test.go b/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig_test.go index 0971e13fc1..e1da6e72fa 100644 --- a/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig_test.go +++ b/private/bufpkg/bufplugin/bufpluginconfig/bufpluginconfig_test.go @@ -27,48 +27,27 @@ import ( func TestGetConfigForBucket(t *testing.T) { t.Parallel() storageosProvider := storageos.NewProvider() - readWriteBucket, err := storageosProvider.NewReadWriteBucket(filepath.Join("testdata", "success", "archive")) + readWriteBucket, err := storageosProvider.NewReadWriteBucket(filepath.Join("testdata", "success", "go")) require.NoError(t, err) pluginConfig, err := GetConfigForBucket(context.Background(), readWriteBucket) require.NoError(t, err) - pluginIdentity, err := bufpluginref.PluginIdentityForString("buf.build/protocolbuffers/java") + pluginIdentity, err := bufpluginref.PluginIdentityForString("buf.build/grpc/go") require.NoError(t, err) require.Equal( t, &Config{ Name: pluginIdentity, - Runtime: &RuntimeConfig{ - Archive: &ArchiveRuntimeConfig{ - Deps: []string{ - "io.grpc:grpc-protobuf:v1.46.0", - "io.grpc:grpc-netty-shaded:v1.46.0", - "io.grpc:grpc-stub:v1.46.0", - "io.grpc:grpc-okhttp:v1.46.0", - }, - }, + Options: map[string]string{ + "paths": "source_relative", }, - }, - pluginConfig, - ) -} - -func TestParsePluginConfigArchiveYAML(t *testing.T) { - t.Parallel() - pluginConfig, err := ParseConfig(filepath.Join("testdata", "success", "archive", "buf.plugin.yaml")) - require.NoError(t, err) - pluginIdentity, err := bufpluginref.PluginIdentityForString("buf.build/protocolbuffers/java") - require.NoError(t, err) - require.Equal( - t, - &Config{ - Name: pluginIdentity, Runtime: &RuntimeConfig{ - Archive: &ArchiveRuntimeConfig{ - Deps: []string{ - "io.grpc:grpc-protobuf:v1.46.0", - "io.grpc:grpc-netty-shaded:v1.46.0", - "io.grpc:grpc-stub:v1.46.0", - "io.grpc:grpc-okhttp:v1.46.0", + Go: &GoRuntimeConfig{ + MinVersion: "1.18", + Deps: []*GoRuntimeDependencyConfig{ + { + Module: "google.golang.org/grpc", + Version: "v1.32.0", + }, }, }, }, @@ -92,9 +71,12 @@ func TestParsePluginConfigGoYAML(t *testing.T) { }, Runtime: &RuntimeConfig{ Go: &GoRuntimeConfig{ - MinVersion: "v1.18", - Deps: []string{ - "google.golang.org/grpc:v1.32.0", + MinVersion: "1.18", + Deps: []*GoRuntimeDependencyConfig{ + { + Module: "google.golang.org/grpc", + Version: "v1.32.0", + }, }, }, }, @@ -118,9 +100,15 @@ func TestParsePluginConfigNPMYAML(t *testing.T) { }, Runtime: &RuntimeConfig{ NPM: &NPMRuntimeConfig{ - Deps: []string{ - "grpc-web:v1.3.1", - "@types/google-protobuf:v3.15.6", + Deps: []*NPMRuntimeDependencyConfig{ + { + Package: "grpc-web", + Version: "^1.3.1", + }, + { + Package: "@types/google-protobuf", + Version: "^3.15.6", + }, }, }, }, diff --git a/private/bufpkg/bufplugin/bufpluginconfig/config.go b/private/bufpkg/bufplugin/bufpluginconfig/config.go index 1747e86a75..507c4455f2 100644 --- a/private/bufpkg/bufplugin/bufpluginconfig/config.go +++ b/private/bufpkg/bufplugin/bufpluginconfig/config.go @@ -20,15 +20,7 @@ import ( "strings" "github.com/bufbuild/buf/private/bufpkg/bufplugin/bufpluginref" - // Note that the semver package we're using conforms to the - // support SemVer syntax found in the go.mod file. This means - // that runtime dependencies will need to specify the 'v' prefix - // in their semantic version even if it isn't directly applicable - // to that runtime environment (e.g. NPM). - // - // We'll use this for now so that runtime dependencies are - // consistent across each runtime configuration, but we might need - // to change this later. + "golang.org/x/mod/modfile" "golang.org/x/mod/semver" ) @@ -83,15 +75,28 @@ func newConfig(externalConfig ExternalConfig) (*Config, error) { func newRuntimeConfig(externalRuntimeConfig ExternalRuntimeConfig) (*RuntimeConfig, error) { var ( - isArchiveEmpty = externalRuntimeConfig.Archive.IsEmpty() - isGoEmpty = externalRuntimeConfig.Go.IsEmpty() - isNPMEmpty = externalRuntimeConfig.NPM.IsEmpty() + isGoEmpty = externalRuntimeConfig.Go.IsEmpty() + isNPMEmpty = externalRuntimeConfig.NPM.IsEmpty() ) - if isArchiveEmpty && isGoEmpty && isNPMEmpty { + var runtimeCount int + for _, isEmpty := range []bool{ + isGoEmpty, + isNPMEmpty, + } { + if !isEmpty { + runtimeCount++ + } + if runtimeCount > 1 { + // We might eventually want to support multiple runtime configuration, + // but it's safe to start with an error for now. + return nil, fmt.Errorf("%s configuration contains multiple runtime languages", ExternalConfigFilePath) + } + } + if runtimeCount == 0 { // It's possible that the plugin doesn't have any runtime dependencies. return nil, nil } - if isArchiveEmpty && isGoEmpty && !isNPMEmpty { + if !isNPMEmpty { npmRuntimeConfig, err := newNPMRuntimeConfig(externalRuntimeConfig.NPM) if err != nil { return nil, err @@ -100,92 +105,73 @@ func newRuntimeConfig(externalRuntimeConfig ExternalRuntimeConfig) (*RuntimeConf NPM: npmRuntimeConfig, }, nil } - if isArchiveEmpty && !isGoEmpty && isNPMEmpty { - goRuntimeConfig, err := newGoRuntimeConfig(externalRuntimeConfig.Go) - if err != nil { - return nil, err - } - return &RuntimeConfig{ - Go: goRuntimeConfig, - }, nil - } - if !isArchiveEmpty && isGoEmpty && isNPMEmpty { - archiveRuntimeConfig, err := newArchiveRuntimeConfig(externalRuntimeConfig.Archive) - if err != nil { - return nil, err - } - return &RuntimeConfig{ - Archive: archiveRuntimeConfig, - }, nil + // At this point, the Go runtime is guaranteed to be specified. Note + // that this will change if/when there are more runtime languages supported. + goRuntimeConfig, err := newGoRuntimeConfig(externalRuntimeConfig.Go) + if err != nil { + return nil, err } - // If we made it this far, that means the config specifies multiple - // runtime languages. - // - // We might eventually want to support multiple runtime configuration - // (e.g. 'go' and 'archive'), but it's safe to start with an error for - // now. - return nil, fmt.Errorf("%s configuration contains multiple runtime languages", ExternalConfigFilePath) + return &RuntimeConfig{ + Go: goRuntimeConfig, + }, nil } func newNPMRuntimeConfig(externalNPMRuntimeConfig ExternalNPMRuntimeConfig) (*NPMRuntimeConfig, error) { - if err := validateRuntimeDeps(externalNPMRuntimeConfig.Deps); err != nil { - return nil, err + var dependencies []*NPMRuntimeDependencyConfig + for _, dep := range externalNPMRuntimeConfig.Deps { + if dep.Package == "" { + return nil, errors.New("npm runtime dependency requires a non-empty package name") + } + if dep.Version == "" { + return nil, errors.New("npm runtime dependency requires a non-empty version name") + } + // TODO: Note that we don't have NPM-specific validation yet - any + // non-empty string will work for the package and version. + // + // For a complete set of the version syntax we need to support, see + // https://docs.npmjs.com/cli/v6/using-npm/semver + // + // https://github.com/Masterminds/semver might be a good candidate for + // this, but it might not support all of the constraints supported + // by NPM. + dependencies = append( + dependencies, + &NPMRuntimeDependencyConfig{ + Package: dep.Package, + Version: dep.Version, + }, + ) } return &NPMRuntimeConfig{ - Deps: externalNPMRuntimeConfig.Deps, + Deps: dependencies, }, nil } func newGoRuntimeConfig(externalGoRuntimeConfig ExternalGoRuntimeConfig) (*GoRuntimeConfig, error) { - if err := validateRuntimeDeps(externalGoRuntimeConfig.Deps); err != nil { - return nil, err + if externalGoRuntimeConfig.MinVersion != "" && !modfile.GoVersionRE.MatchString(externalGoRuntimeConfig.MinVersion) { + return nil, fmt.Errorf("the go minimum version %q must be a valid semantic version in the form of .", externalGoRuntimeConfig.MinVersion) } - // The best we can do is verify that the minimum version - // is a valid semantic version, just like we do for the - // runtime dependencies. - // - // This will not actually verify that the go version is - // in the valid set. It's impossible to capture the - // real set of valid identifiers at any given time (for - // an old version of the buf CLI) without reaching out to - // some external source at runtime. - // - // Note that this ensures the user's configuration specifies - // a 'v' prefix in the version (e.g. v1.18) even though the - // minimum version is rendered without it in the go.mod. - if externalGoRuntimeConfig.MinVersion != "" && !semver.IsValid(externalGoRuntimeConfig.MinVersion) { - return nil, fmt.Errorf("the go minimum version %q must be a valid semantic version", externalGoRuntimeConfig.MinVersion) - } - return &GoRuntimeConfig{ - MinVersion: externalGoRuntimeConfig.MinVersion, - Deps: externalGoRuntimeConfig.Deps, - }, nil -} - -func newArchiveRuntimeConfig(externalArchiveRuntimeConfig ExternalArchiveRuntimeConfig) (*ArchiveRuntimeConfig, error) { - if err := validateRuntimeDeps(externalArchiveRuntimeConfig.Deps); err != nil { - return nil, err - } - return &ArchiveRuntimeConfig{ - Deps: externalArchiveRuntimeConfig.Deps, - }, nil -} - -func validateRuntimeDeps(dependencies []string) error { - seen := make(map[string]struct{}, len(dependencies)) - for _, dependency := range dependencies { - split := strings.Split(dependency, ":") - if len(split) < 2 { - return fmt.Errorf(`runtime dependency %q must be specified as ":"`, dependency) + var dependencies []*GoRuntimeDependencyConfig + for _, dep := range externalGoRuntimeConfig.Deps { + if dep.Module == "" { + return nil, errors.New("go runtime dependency requires a non-empty module name") } - name, version := strings.Join(split[:len(split)-1], ":"), split[len(split)-1] - if _, ok := seen[name]; ok { - return fmt.Errorf("runtime dependency %q was specified more than once", name) + if dep.Version == "" { + return nil, errors.New("go runtime dependency requires a non-empty version name") } - if !semver.IsValid(version) { - return fmt.Errorf("runtime dependency %q does not have a valid semantic version", dependency) + if !semver.IsValid(dep.Version) { + return nil, fmt.Errorf("go runtime dependency %s:%s does not have a valid semantic version", dep.Module, dep.Version) } - seen[name] = struct{}{} + dependencies = append( + dependencies, + &GoRuntimeDependencyConfig{ + Module: dep.Module, + Version: dep.Version, + }, + ) } - return nil + return &GoRuntimeConfig{ + MinVersion: externalGoRuntimeConfig.MinVersion, + Deps: dependencies, + }, nil } diff --git a/private/bufpkg/bufplugin/bufpluginconfig/testdata/failure/invalid-multiple-languages.yaml b/private/bufpkg/bufplugin/bufpluginconfig/testdata/failure/invalid-multiple-languages.yaml index 90c943b5d5..22bf5e64b0 100644 --- a/private/bufpkg/bufplugin/bufpluginconfig/testdata/failure/invalid-multiple-languages.yaml +++ b/private/bufpkg/bufplugin/bufpluginconfig/testdata/failure/invalid-multiple-languages.yaml @@ -5,6 +5,7 @@ opts: runtime: npm: deps: - - "@types/google-protobuf:v3.15.6" + - package: @types/google-protobuf + version: ^3.15.6 go: min_version: 1.18 diff --git a/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/archive/buf.plugin.yaml b/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/archive/buf.plugin.yaml deleted file mode 100644 index 3af777a333..0000000000 --- a/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/archive/buf.plugin.yaml +++ /dev/null @@ -1,9 +0,0 @@ -version: v1 -name: buf.build/protocolbuffers/java -runtime: - archive: - deps: - - "io.grpc:grpc-protobuf:v1.46.0" - - "io.grpc:grpc-netty-shaded:v1.46.0" - - "io.grpc:grpc-stub:v1.46.0" - - "io.grpc:grpc-okhttp:v1.46.0" diff --git a/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/go/buf.plugin.yaml b/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/go/buf.plugin.yaml index de31edb08d..6abf261988 100644 --- a/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/go/buf.plugin.yaml +++ b/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/go/buf.plugin.yaml @@ -4,6 +4,7 @@ opts: - paths=source_relative runtime: go: - min_version: v1.18 + min_version: 1.18 deps: - - "google.golang.org/grpc:v1.32.0" + - module: google.golang.org/grpc + version: v1.32.0 diff --git a/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/npm/buf.plugin.yaml b/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/npm/buf.plugin.yaml index f4303e0432..b9510b6e43 100644 --- a/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/npm/buf.plugin.yaml +++ b/private/bufpkg/bufplugin/bufpluginconfig/testdata/success/npm/buf.plugin.yaml @@ -5,5 +5,7 @@ opts: runtime: npm: deps: - - "grpc-web:v1.3.1" - - "@types/google-protobuf:v3.15.6" + - package: "grpc-web" + version: "^1.3.1" + - package: "@types/google-protobuf" + version: "^3.15.6"