diff --git a/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/buf.lock b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/buf.lock new file mode 100644 index 0000000000..079754907b --- /dev/null +++ b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/buf.lock @@ -0,0 +1,5 @@ +version: v2 +deps: + - name: buf.testing/acme/date + commit: ffded0b4cf6b47cab74da08d291a3c2f + digest: b5:24ed4f13925cf89ea0ae0127fa28540704c7ae14750af027270221b737a1ce658f8014ca2555f6f7fcd95ea84e071d33f37f86cc36d07fe0d0963329a5ec2462 diff --git a/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/buf.yaml b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/buf.yaml new file mode 100644 index 0000000000..ce1db00103 --- /dev/null +++ b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/buf.yaml @@ -0,0 +1,7 @@ +version: v2 +modules: + - path: finance/bond/proto + name: buf.testing/acme/bond +deps: + - buf.testing/acme/date + - buf.testing/acme/extension diff --git a/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/finance/bond/proto/acme/bond/v2/bond.proto b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/finance/bond/proto/acme/bond/v2/bond.proto new file mode 100644 index 0000000000..53e0e25f8f --- /dev/null +++ b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock/finance/bond/proto/acme/bond/v2/bond.proto @@ -0,0 +1,10 @@ +syntax = "proto3"; + +package acme.bond.v2; + +import "google/protobuf/duration.proto"; + +message Bond { + google.protobuf.Duration duration = 4; + string name = 6; +} diff --git a/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/buf.lock b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/buf.lock new file mode 100644 index 0000000000..079754907b --- /dev/null +++ b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/buf.lock @@ -0,0 +1,5 @@ +version: v2 +deps: + - name: buf.testing/acme/date + commit: ffded0b4cf6b47cab74da08d291a3c2f + digest: b5:24ed4f13925cf89ea0ae0127fa28540704c7ae14750af027270221b737a1ce658f8014ca2555f6f7fcd95ea84e071d33f37f86cc36d07fe0d0963329a5ec2462 diff --git a/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/buf.yaml b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/buf.yaml new file mode 100644 index 0000000000..ce1db00103 --- /dev/null +++ b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/buf.yaml @@ -0,0 +1,7 @@ +version: v2 +modules: + - path: finance/bond/proto + name: buf.testing/acme/bond +deps: + - buf.testing/acme/date + - buf.testing/acme/extension diff --git a/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/finance/bond/proto/acme/bond/v2/bond.proto b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/finance/bond/proto/acme/bond/v2/bond.proto new file mode 100644 index 0000000000..519969b470 --- /dev/null +++ b/private/buf/bufworkspace/testdata/basic/workspace_dep_missing_from_lock_used/finance/bond/proto/acme/bond/v2/bond.proto @@ -0,0 +1,11 @@ +syntax = "proto3"; + +package acme.bond.v2; + +import "acme/extension/v1/extension.proto"; + +message Bond { + option (acme.extension.v1.experimental) = true; + + string name = 1; +} diff --git a/private/buf/bufworkspace/workspace_provider.go b/private/buf/bufworkspace/workspace_provider.go index 65a97c0011..8d0ea4b28f 100644 --- a/private/buf/bufworkspace/workspace_provider.go +++ b/private/buf/bufworkspace/workspace_provider.go @@ -20,6 +20,7 @@ import ( "fmt" "io/fs" "log/slog" + "slices" "buf.build/go/standard/xlog/xslog" "buf.build/go/standard/xslices" @@ -323,6 +324,8 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( v1WorkspaceTargeting *v1Targeting, ) (*workspace, error) { moduleSetBuilder := bufmodule.NewModuleSetBuilder(ctx, w.logger, w.moduleDataProvider, w.commitProvider) + var hadBufLock bool + lockModuleFullNames := make(map[string]struct{}) for _, moduleBucketAndTargeting := range v1WorkspaceTargeting.moduleBucketsAndTargeting { mappedModuleBucket := moduleBucketAndTargeting.bucket moduleTargeting := moduleBucketAndTargeting.moduleTargeting @@ -356,6 +359,7 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( default: return nil, syserror.Newf("unknown FileVersion: %v", fileVersion) } + hadBufLock = true for _, depModuleKey := range bufLockFile.DepModuleKeys() { // DepModuleKeys from a BufLockFile is expected to have all transitive dependencies, // and we can rely on this property. @@ -363,6 +367,7 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( depModuleKey, false, ) + lockModuleFullNames[depModuleKey.FullName().String()] = struct{}{} } } v1BufYAMLObjectData, err := bufconfig.GetBufYAMLV1Beta1OrV1ObjectDataForPrefix(ctx, bucket, moduleTargeting.moduleDirPath) @@ -386,6 +391,9 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( // configs, however, we return this error as a safety check return nil, fmt.Errorf("no module config found for module at: %q", moduleTargeting.moduleDirPath) } + if moduleFullName := moduleConfig.FullName(); moduleFullName != nil { + lockModuleFullNames[moduleFullName.String()] = struct{}{} + } moduleSetBuilder.AddLocalModule( mappedModuleBucket, moduleBucketAndTargeting.bucketID, @@ -410,6 +418,11 @@ func (w *workspaceProvider) getWorkspaceForBucketAndModuleDirPathsV1Beta1OrV1( ), ) } + if hadBufLock { + if missingFromLock := depsInYAMLMissingFromLock(v1WorkspaceTargeting.allConfiguredDepModuleRefs, lockModuleFullNames); len(missingFromLock) > 0 { + return nil, fmt.Errorf("%s declared in buf.yaml deps but not present in buf.lock; run \"buf dep update\" to update the lock file", xstrings.SliceToHumanStringQuoted(missingFromLock)) + } + } moduleSet, err := moduleSetBuilder.Build() if err != nil { return nil, err @@ -438,6 +451,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2( remotePolicyKeys []bufpolicy.PolicyKey policyNameToRemotePluginKeys map[string][]bufplugin.PluginKey ) + lockModuleFullNames := make(map[string]struct{}) bufLockFile, err := bufconfig.GetBufLockFileForPrefix( ctx, bucket, @@ -465,6 +479,7 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2( depModuleKey, false, ) + lockModuleFullNames[depModuleKey.FullName().String()] = struct{}{} } remotePluginKeys = bufLockFile.RemotePluginKeys() remotePolicyKeys = bufLockFile.RemotePolicyKeys() @@ -502,6 +517,9 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2( return nil, fmt.Errorf("multiple module configs found with the same description: %s", moduleDescription) } seenModuleDescriptions[moduleDescription] = struct{}{} + if moduleFullName := moduleConfig.FullName(); moduleFullName != nil { + lockModuleFullNames[moduleFullName.String()] = struct{}{} + } moduleSetBuilder.AddLocalModule( mappedModuleBucket, moduleBucketAndTargeting.bucketID, @@ -518,6 +536,11 @@ func (w *workspaceProvider) getWorkspaceForBucketBufYAMLV2( bufmodule.LocalModuleWithDescription(moduleDescription), ) } + if bufLockFile != nil { + if missingFromLock := depsInYAMLMissingFromLock(v2Targeting.bufYAMLFile.ConfiguredDepModuleRefs(), lockModuleFullNames); len(missingFromLock) > 0 { + return nil, fmt.Errorf("%s declared in buf.yaml deps but not present in buf.lock; run \"buf dep update\" to update the lock file", xstrings.SliceToHumanStringQuoted(missingFromLock)) + } + } moduleSet, err := moduleSetBuilder.Build() if err != nil { return nil, err @@ -578,6 +601,19 @@ func (w *workspaceProvider) getWorkspaceForBucketModuleSet( ), nil } +// depsInYAMLMissingFromLock returns the sorted list of module full name strings +// that appear in configuredDepModuleRefs but are absent from lockModuleFullNames. +func depsInYAMLMissingFromLock(configuredDepModuleRefs []bufparse.Ref, lockModuleFullNames map[string]struct{}) []string { + var missing []string + for _, ref := range configuredDepModuleRefs { + if _, ok := lockModuleFullNames[ref.FullName().String()]; !ok { + missing = append(missing, ref.FullName().String()) + } + } + slices.Sort(missing) + return missing +} + // This formats a module name based on its module config entry in the v2 buf.yaml: // `path: foo, includes: ["foo/v1, "foo/v2"], excludes: "foo/v1/internal"`. // diff --git a/private/buf/bufworkspace/workspace_test.go b/private/buf/bufworkspace/workspace_test.go index 9224fed707..34203edc54 100644 --- a/private/buf/bufworkspace/workspace_test.go +++ b/private/buf/bufworkspace/workspace_test.go @@ -247,6 +247,84 @@ func TestUnusedDep(t *testing.T) { require.Equal(t, MalformedDepTypeUnused, malformedDeps[1].Type()) } +func TestDepMissingFromLock(t *testing.T) { + t.Parallel() + ctx := t.Context() + + workspaceProvider := testNewWorkspaceProvider( + t, + bufmoduletesting.ModuleData{ + Name: "buf.testing/acme/date", + DirPath: "testdata/basic/bsr/buf.testing/acme/date", + }, + bufmoduletesting.ModuleData{ + Name: "buf.testing/acme/extension", + DirPath: "testdata/basic/bsr/buf.testing/acme/extension", + }, + ) + + storageosProvider := storageos.NewProvider() + bucket, err := storageosProvider.NewReadWriteBucket("testdata/basic/workspace_dep_missing_from_lock") + require.NoError(t, err) + bucketTargeting, err := buftarget.NewBucketTargeting( + ctx, + slogtestext.NewLogger(t), + bucket, + ".", + nil, + nil, + buftarget.TerminateAtControllingWorkspace, + ) + require.NoError(t, err) + + _, err = workspaceProvider.GetWorkspaceForBucket( + ctx, + bucket, + bucketTargeting, + ) + require.ErrorContains(t, err, `"buf.testing/acme/extension" declared in buf.yaml deps but not present in buf.lock`) + require.ErrorContains(t, err, `buf dep update`) +} + +func TestDepMissingFromLockUsed(t *testing.T) { + t.Parallel() + ctx := t.Context() + + workspaceProvider := testNewWorkspaceProvider( + t, + bufmoduletesting.ModuleData{ + Name: "buf.testing/acme/date", + DirPath: "testdata/basic/bsr/buf.testing/acme/date", + }, + bufmoduletesting.ModuleData{ + Name: "buf.testing/acme/extension", + DirPath: "testdata/basic/bsr/buf.testing/acme/extension", + }, + ) + + storageosProvider := storageos.NewProvider() + bucket, err := storageosProvider.NewReadWriteBucket("testdata/basic/workspace_dep_missing_from_lock_used") + require.NoError(t, err) + bucketTargeting, err := buftarget.NewBucketTargeting( + ctx, + slogtestext.NewLogger(t), + bucket, + ".", + nil, + nil, + buftarget.TerminateAtControllingWorkspace, + ) + require.NoError(t, err) + + _, err = workspaceProvider.GetWorkspaceForBucket( + ctx, + bucket, + bucketTargeting, + ) + require.ErrorContains(t, err, `"buf.testing/acme/extension" declared in buf.yaml deps but not present in buf.lock`) + require.ErrorContains(t, err, `buf dep update`) +} + func TestDuplicatePath(t *testing.T) { t.Parallel() ctx := t.Context()