diff --git a/cmd/generate/client.go b/cmd/generate/client.go index b05b51d2..a4fa1fb7 100644 --- a/cmd/generate/client.go +++ b/cmd/generate/client.go @@ -4,7 +4,6 @@ import ( "context" "crypto/sha256" "fmt" - "os" "path/filepath" "strings" "time" @@ -376,7 +375,10 @@ func buildLocalContractEndpoint(ctx context.Context, service *resources.Service, if ok, _ := shared.FileExists(ctx, filepath.Join(protoDir, "buf.yaml")); !ok { return nil, nil, nil, fmt.Errorf("service %s has no proto/buf.yaml; cannot build a descriptor set for endpoint %s", service.Name, endpoint.Name) } - descriptorSet, err := buildDescriptorSet(ctx, protoDir) + // buf compiles the descriptor set and names its own source set in the + // same companion run, so Sources are exactly the files and import names + // the descriptor set was built from. + descriptorSet, protoSources, err := compileProtoDir(ctx, protoDir, true) if err != nil { return nil, nil, nil, fmt.Errorf("cannot build descriptor set: %w", err) } @@ -384,10 +386,6 @@ func buildLocalContractEndpoint(ctx context.Context, service *resources.Service, if err = googleproto.Unmarshal(descriptorSet, &set); err != nil { return nil, nil, nil, fmt.Errorf("cannot parse generated descriptor set: %w", err) } - protoSources, err := collectProtoSources(protoDir) - if err != nil { - return nil, nil, nil, fmt.Errorf("cannot read proto sources: %w", err) - } // The contract's package is the one the service's own protos declare // their services in, as `generate contracts` settles it; an endpoint // declaring none in service.codefly.yaml is the common case. @@ -431,36 +429,6 @@ func buildLocalContractEndpoint(ctx context.Context, service *resources.Service, return nil, nil, nil, fmt.Errorf("endpoint %s reports neither a grpc nor a rest contract", endpoint.Name) } -// collectProtoSources reads every .proto file directly (and doubly) under -// protoDir into proto.GenerateClient's Sources form, path relative to -// protoDir — the same files buildDescriptorSet compiled, so generation from -// Sources produces the same code the persisted descriptor set describes. -func collectProtoSources(protoDir string) ([]coreproto.Source, error) { - var sources []coreproto.Source - err := filepath.WalkDir(protoDir, func(p string, d os.DirEntry, err error) error { - if err != nil || d.IsDir() || !strings.HasSuffix(p, ".proto") { - return err - } - rel, err := filepath.Rel(protoDir, p) - if err != nil { - return err - } - content, err := os.ReadFile(p) //nolint:gosec // p is walked from a service's own committed proto directory - if err != nil { - return err - } - sources = append(sources, coreproto.Source{Path: filepath.ToSlash(rel), Content: content}) - return nil - }) - if err != nil { - return nil, err - } - if len(sources) == 0 { - return nil, fmt.Errorf("no .proto files under %s", protoDir) - } - return sources, nil -} - // resolvePackageSource handles --from package:@: it finds the // composed module whose package manifest ID matches id, requires it to be // pinned at exactly version, and reads its committed API contract catalog. diff --git a/cmd/generate/client_proto_sources_companion_test.go b/cmd/generate/client_proto_sources_companion_test.go new file mode 100644 index 00000000..b79a7b21 --- /dev/null +++ b/cmd/generate/client_proto_sources_companion_test.go @@ -0,0 +1,71 @@ +//go:build proto_companion_required + +package generate + +import ( + "os" + "path/filepath" + "sort" + "testing" + + "github.com/stretchr/testify/require" +) + +// sourceNames is the source set buf names for a proto tree, through the +// pinned companion. +func sourceNames(t *testing.T, dir string) []string { + t.Helper() + _, sources, err := compileProtoDir(t.Context(), dir, true) + require.NoError(t, err) + var names []string + for _, source := range sources { + names = append(names, source.Path) + } + sort.Strings(names) + return names +} + +func writeProtos(t *testing.T, root string, rels ...string) { + t.Helper() + for _, rel := range rels { + writeTestFile(t, filepath.Join(root, rel), "syntax = \"proto3\";\n") + } +} + +// A v2 buf.yaml with a vendor module and an excluded tree: every source is +// named relative to the module that holds it, and an excluded tree is not a +// source. +func TestBufNamesV2ModulesAndExcludesThroughPinnedCompanion(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v2\nmodules:\n - path: .\n excludes: [facade, vendor]\n - path: vendor\n") + writeProtos(t, dir, "acme/v1/api.proto", "facade/api.proto", "vendor/upstream/base/v0/base.proto") + require.Equal(t, []string{"acme/v1/api.proto", "upstream/base/v0/base.proto"}, sourceNames(t, dir)) +} + +// A v1 buf.yaml's build.excludes keep a tree out of the source set. +func TestBufNamesV1BuildExcludesThroughPinnedCompanion(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v1\nbuild:\n excludes:\n - facade\n") + writeProtos(t, dir, "acme/v1/api.proto", "facade/api.proto") + require.Equal(t, []string{"acme/v1/api.proto"}, sourceNames(t, dir)) +} + +// A v2 module's includes narrow it to the trees named. +func TestBufNamesV2IncludesThroughPinnedCompanion(t *testing.T) { + dir := t.TempDir() + writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v2\nmodules:\n - path: .\n includes: [acme]\n") + writeProtos(t, dir, "acme/v1/api.proto", "other/v1/other.proto") + require.Equal(t, []string{"acme/v1/api.proto"}, sourceNames(t, dir)) +} + +// A module folder that is a symlink contributes its files under their import +// names: the link's target is copied in, since it is not mounted. +func TestBufNamesASymlinkedModuleThroughPinnedCompanion(t *testing.T) { + root := t.TempDir() + dir, shared := filepath.Join(root, "proto"), filepath.Join(root, "shared") + writeProtos(t, shared, "upstream/base/v0/base.proto") + writeTestFile(t, filepath.Join(dir, "buf.yaml"), "version: v2\nmodules:\n - path: .\n excludes: [vendor]\n - path: vendor\n") + writeProtos(t, dir, "acme/v1/api.proto") + require.NoError(t, os.Symlink(shared, filepath.Join(dir, "vendor"))) + require.Equal(t, []string{"acme/v1/api.proto", "upstream/base/v0/base.proto"}, sourceNames(t, dir)) +} diff --git a/cmd/generate/client_proto_sources_test.go b/cmd/generate/client_proto_sources_test.go new file mode 100644 index 00000000..e7dbf70d --- /dev/null +++ b/cmd/generate/client_proto_sources_test.go @@ -0,0 +1,90 @@ +package generate + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// buf names the source set: each file is read from the path buf found it at +// and named by its import path, and a file buf lists only as an import is not +// a source. +func TestProtoSourcesFromListingNamesByImportPath(t *testing.T) { + root := t.TempDir() + for _, rel := range []string{"acme/v1/api.proto", "vendor/upstream/base/v0/base.proto"} { + writeTestFile(t, filepath.Join(root, rel), "syntax = \"proto3\";\n// "+rel+"\n") + } + listing := []byte(`{"path":"acme/v1/api.proto","import_path":"acme/v1/api.proto","is_import":false} +{"path":"vendor/upstream/base/v0/base.proto","import_path":"upstream/base/v0/base.proto","is_import":false} +{"path":"buf/validate/validate.proto","import_path":"buf/validate/validate.proto","is_import":true} +`) + sources, err := protoSourcesFromListing(root, listing) + if err != nil { + t.Fatal(err) + } + if len(sources) != 2 || sources[0].Path != "acme/v1/api.proto" || sources[1].Path != "upstream/base/v0/base.proto" { + t.Fatalf("sources = %+v", sources) + } + if string(sources[1].Content) != "syntax = \"proto3\";\n// vendor/upstream/base/v0/base.proto\n" { + t.Fatalf("content read from the wrong file: %q", sources[1].Content) + } +} + +// A listed path that leaves the copied tree, or a listing with nothing to +// compile, is refused rather than read. +func TestProtoSourcesFromListingRefusesWhatItCannotRead(t *testing.T) { + root := t.TempDir() + if _, err := protoSourcesFromListing(root, []byte(`{"path":"../escape.proto","import_path":"escape.proto"}`)); err == nil { + t.Fatal("a path outside the tree was read") + } + if _, err := protoSourcesFromListing(root, nil); err == nil { + t.Fatal("an empty listing was accepted") + } + if _, err := os.Stat(filepath.Join(root, "escape.proto")); !os.IsNotExist(err) { + t.Fatal(err) + } +} + +// A linked folder outside the tree is copied as what it holds; a link to a +// folder that holds it, or to one already being copied, is refused by name +// rather than recursed into. +func TestCopyTreeFollowsLinkedFoldersAndRefusesLoops(t *testing.T) { + root := t.TempDir() + src, shared := filepath.Join(root, "proto"), filepath.Join(root, "shared") + writeTestFile(t, filepath.Join(src, "acme/v1/api.proto"), "a") + writeTestFile(t, filepath.Join(shared, "upstream/base.proto"), "b") + if err := os.Symlink(shared, filepath.Join(src, "vendor")); err != nil { + t.Fatal(err) + } + dst := filepath.Join(t.TempDir(), "copy") + if err := copyTree(src, dst); err != nil { + t.Fatal(err) + } + if data, err := os.ReadFile(filepath.Join(dst, "vendor/upstream/base.proto")); err != nil || string(data) != "b" { + t.Fatalf("linked folder not copied: %q, %v", data, err) + } + + for name, link := range map[string]struct{ at, to string }{ + "to its own folder": {"acme/loop", "acme"}, + "to the copied root": {"acme/v1/up", "."}, + "back through a link": {"../shared/upstream/back", "."}, + "to a folder above root": {"acme/out", ".."}, + } { + t.Run(name, func(t *testing.T) { + at := filepath.Join(src, filepath.FromSlash(link.at)) + if err := os.Symlink(filepath.Join(src, link.to), at); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Remove(at) }) + err := copyTree(src, filepath.Join(t.TempDir(), "copy")) + if err == nil || !strings.Contains(err.Error(), "looping link") { + t.Fatalf("copyTree = %v, want a looping link refused", err) + } + resolved, _ := filepath.EvalSymlinks(at) + if !strings.Contains(err.Error(), resolved) { + t.Fatalf("refusal %q does not name the link's target %s", err, resolved) + } + }) + } +} diff --git a/cmd/generate/contracts.go b/cmd/generate/contracts.go index 5eb8c5cc..66924da7 100644 --- a/cmd/generate/contracts.go +++ b/cmd/generate/contracts.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "path/filepath" "sort" @@ -656,22 +657,40 @@ func printContractLine(endpoint *composition.APIContractEndpoint) { // included. It works from a temporary copy of protoDir so buf's dependency // resolution never mutates the service's committed buf.lock. func buildDescriptorSet(ctx context.Context, protoDir string) ([]byte, error) { + descriptor, _, err := compileProtoDir(ctx, protoDir, false) + return descriptor, err +} + +// ListProtoSources names protoDir's source set as buf compiles it: the one +// source-set mechanism, shared by `generate client` and `publish clients` +// (over a published contract's proto tree, which carries its buf.yaml). +func ListProtoSources(ctx context.Context, protoDir string) ([]proto.Source, error) { + _, sources, err := compileProtoDir(ctx, protoDir, true) + return sources, err +} + +// compileProtoDir has buf, in the pinned proto companion, build protoDir's +// descriptor set and, when listSources is set, name the source set it built +// it from (`buf ls-files --format json`): every file buf compiles, read from +// where buf found it and named by its import path. buf.yaml's modules, +// includes, excludes and symlinks are buf's to interpret, never this CLI's. +func compileProtoDir(ctx context.Context, protoDir string, listSources bool) ([]byte, []proto.Source, error) { if !runners.DockerEngineRunning(ctx) { - return nil, fmt.Errorf("docker is not running; please start Docker first") + return nil, nil, fmt.Errorf("docker is not running; please start Docker first") } image, err := proto.CompanionImage(ctx) if err != nil { - return nil, fmt.Errorf("cannot get companion image: %w", err) + return nil, nil, fmt.Errorf("cannot get companion image: %w", err) } tmpDir, err := os.MkdirTemp("", "codefly-contract-proto") if err != nil { - return nil, err + return nil, nil, err } defer func() { _ = os.RemoveAll(tmpDir) }() if err = copyTree(protoDir, tmpDir); err != nil { - return nil, fmt.Errorf("cannot copy proto sources: %w", err) + return nil, nil, fmt.Errorf("cannot copy proto sources: %w", err) } projectContainerRecovery(ctx) @@ -679,7 +698,7 @@ func buildDescriptorSet(ctx context.Context, protoDir string) ([]byte, error) { name := fmt.Sprintf("contracts-%d", time.Now().UnixMilli()) runner, err := runners.NewDockerEnvironment(ctx, image, tmpDir, name) if err != nil { - return nil, fmt.Errorf("cannot create docker runner: %w", err) + return nil, nil, fmt.Errorf("cannot create docker runner: %w", err) } // A descriptor-set container holds no state worth preserving across runs: it // is created, driven once and shut down in the defer below. Marking it @@ -705,41 +724,137 @@ func buildDescriptorSet(ctx context.Context, protoDir string) ([]byte, error) { }() if err = runner.Init(ctx); err != nil { - return nil, fmt.Errorf("cannot init runner: %w", err) + return nil, nil, fmt.Errorf("cannot init runner: %w", err) } depProc, err := runner.NewProcess("buf", "dep", "update") if err != nil { - return nil, fmt.Errorf("cannot create process: %w", err) + return nil, nil, fmt.Errorf("cannot create process: %w", err) } if err = depProc.Run(ctx); err != nil { - return nil, fmt.Errorf("cannot update buf dependencies: %w", err) + return nil, nil, fmt.Errorf("cannot update buf dependencies: %w", err) } const descriptorFile = ".codefly-contract.binpb" buildProc, err := runner.NewProcess("buf", "build", "--as-file-descriptor-set", "-o", "/workspace/"+descriptorFile) if err != nil { - return nil, fmt.Errorf("cannot create process: %w", err) + return nil, nil, fmt.Errorf("cannot create process: %w", err) } if err = buildProc.Run(ctx); err != nil { - return nil, fmt.Errorf("cannot build descriptor set: %w", err) + return nil, nil, fmt.Errorf("cannot build descriptor set: %w", err) } - return os.ReadFile(filepath.Join(tmpDir, descriptorFile)) + descriptor, err := os.ReadFile(filepath.Join(tmpDir, descriptorFile)) + if err != nil || !listSources { + return descriptor, nil, err + } + lsProc, err := runner.NewProcess("buf", "ls-files", "--format", "json") + if err != nil { + return nil, nil, fmt.Errorf("cannot create process: %w", err) + } + stdout, err := lsProc.StdoutPipe() + if err != nil { + return nil, nil, fmt.Errorf("cannot read buf ls-files: %w", err) + } + if err = lsProc.Start(ctx); err != nil { + return nil, nil, fmt.Errorf("cannot list proto sources: %w", err) + } + listing, readErr := io.ReadAll(stdout) + if err = lsProc.Wait(ctx); err != nil { + return nil, nil, fmt.Errorf("cannot list proto sources: %w", err) + } + if readErr != nil { + return nil, nil, fmt.Errorf("cannot read buf ls-files: %w", readErr) + } + sources, err := protoSourcesFromListing(tmpDir, listing) + return descriptor, sources, err +} + +// bufListedFile is one line of `buf ls-files --format json`. +type bufListedFile struct { + Path string `json:"path"` + ImportPath string `json:"import_path"` + IsImport bool `json:"is_import"` +} + +// protoSourcesFromListing reads every file buf listed, from its path under +// root, as a Source named by its import path — the name every importer spells +// and the descriptor set records. Files buf lists only as imports are not +// sources. +func protoSourcesFromListing(root string, listing []byte) ([]proto.Source, error) { + var sources []proto.Source + decoder := json.NewDecoder(bytes.NewReader(listing)) + for decoder.More() { + var file bufListedFile + if err := decoder.Decode(&file); err != nil { + return nil, fmt.Errorf("cannot parse buf ls-files: %w", err) + } + if file.IsImport { + continue + } + if file.Path == "" || file.ImportPath == "" || !filepath.IsLocal(filepath.FromSlash(file.Path)) { + return nil, fmt.Errorf("buf ls-files listed an unusable file %q (%q)", file.Path, file.ImportPath) + } + content, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(file.Path))) //nolint:gosec // a local path buf listed under root + if err != nil { + return nil, err + } + sources = append(sources, proto.Source{Path: file.ImportPath, Content: content}) + } + if len(sources) == 0 { + return nil, fmt.Errorf("buf lists no .proto files to compile") + } + sort.Slice(sources, func(i, j int) bool { return sources[i].Path < sources[j].Path }) + return sources, nil } // copyTree copies the file tree rooted at src into dst, creating dst as -// needed. +// needed. A symlink is copied as what it points at, so a symlinked module +// folder reaches buf in the container, where the link's target is not mounted, +// and the published contract's proto tree is self-contained. A linked folder +// may live outside src (a module shared between services is the reason to link +// one), but a link to a folder that holds it, or to one already being copied, +// is a loop and is refused by name. func copyTree(src, dst string) error { - return filepath.WalkDir(src, func(p string, entry os.DirEntry, walkErr error) error { + root, err := filepath.EvalSymlinks(src) + if err != nil { + return err + } + return copyResolvedTree(root, dst, []string{root}) +} + +// copyResolvedTree copies root, a directory with no symlink in its path, into +// dst. copying holds every resolved folder whose copy is in progress, outermost +// first and root last. +func copyResolvedTree(root, dst string, copying []string) error { + return filepath.WalkDir(root, func(p string, entry os.DirEntry, walkErr error) error { if walkErr != nil { return walkErr } - rel, err := filepath.Rel(src, p) + rel, err := filepath.Rel(root, p) if err != nil { return err } target := filepath.Join(dst, rel) + if entry.Type()&os.ModeSymlink != 0 { + info, statErr := os.Stat(p) + if statErr != nil { + return statErr + } + if info.IsDir() { + resolved, linkErr := filepath.EvalSymlinks(p) + if linkErr != nil { + return linkErr + } + // WalkDir follows no link, so p's folder is already resolved. + for _, held := range append([]string{filepath.Dir(p)}, copying...) { + if folderHolds(resolved, held) { + return fmt.Errorf("symlink %s points at %s, which holds it: a looping link cannot be copied", p, resolved) + } + } + return copyResolvedTree(resolved, target, append(copying[:len(copying):len(copying)], resolved)) + } + } if entry.IsDir() { return os.MkdirAll(target, 0o755) } @@ -751,6 +866,12 @@ func copyTree(src, dst string) error { }) } +// folderHolds reports whether dir is folder or lies beneath it. +func folderHolds(folder, dir string) bool { + rel, err := filepath.Rel(folder, dir) + return err == nil && filepath.IsLocal(rel) +} + // runContractsCheck regenerates contracts into a throwaway directory and // diffs them against output, without writing anything real. func runContractsCheck(ctx context.Context, workspace *resources.Workspace, module *resources.Module, output string) error { diff --git a/cmd/generate/proto.go b/cmd/generate/proto.go index 1703c139..313e3d3a 100644 --- a/cmd/generate/proto.go +++ b/cmd/generate/proto.go @@ -945,9 +945,14 @@ func protoGenerationPathArgs(protoDir string, absolute bool) []string { return args } -// A positional Buf input replaces the template's inputs, including their path -// filters. Supply the default only when the template leaves input selection to -// the caller. Relative template inputs resolve in the template's working dir. +// The template's inputs are used as declared, with their input-level options +// (paths, types, exclude_paths), unless a caller's --path meets more than one +// of them. buf applies a --path flag to every input, and an input that does not +// hold the path targets no file and fails the whole run ("no .proto files were +// targeted"), so a caller's --path over several inputs selects --proto, whose +// files it names, as the one input. A positional input replaces the template's, +// as buf itself defines. Relative template inputs resolve in the template's +// working dir. func protoGenerationArgs(protoDir, templatePath string, template []byte) ([]string, error) { var selection struct { Inputs []yaml.Node `yaml:"inputs"` @@ -956,7 +961,7 @@ func protoGenerationArgs(protoDir, templatePath string, template []byte) ([]stri return nil, fmt.Errorf("cannot read generation template inputs: %w", err) } args := []string{"generate"} - if len(selection.Inputs) == 0 { + if len(selection.Inputs) == 0 || (len(protoPaths) > 0 && len(selection.Inputs) > 1) { args = append(args, protoDir) } args = append(args, "--template", templatePath) diff --git a/cmd/generate/proto_companion_test.go b/cmd/generate/proto_companion_test.go index fc7520a1..231b90a1 100644 --- a/cmd/generate/proto_companion_test.go +++ b/cmd/generate/proto_companion_test.go @@ -135,3 +135,44 @@ plugins: require.ErrorIs(t, err, os.ErrNotExist) } } + +// The document store's layout: a template declaring two inputs (its own protos +// with a path selection, and a sibling tree of client contracts), generated +// with a caller --path. buf applies --path to every input, so keeping both +// inputs failed with "no .proto files were targeted"; the caller's selection +// makes --proto the one input. +func TestProtoTwoInputTemplateWithCallerPathThroughPinnedCompanion(t *testing.T) { + root := t.TempDir() + writeTestFile(t, filepath.Join(root, "workspace.codefly.yaml"), "name: example\n") + input := filepath.Join(root, "proto") + for _, dir := range []string{"proto", "deps"} { + writeTestFile(t, filepath.Join(root, dir, "buf.yaml"), "version: v2\nmodules:\n - path: .\n") + } + for _, name := range []string{"proto/api/v1/api", "proto/api/v1/other", "deps/client/v1/client"} { + writeTestFile(t, filepath.Join(root, name+".proto"), `syntax = "proto3"; +package example.v1; +option go_package = "example.com/generated;example"; +message `+filepath.Base(name)+` { string value = 1; } +`) + } + writeTestFile(t, filepath.Join(input, "buf.gen.yaml"), `version: v2 +inputs: + - directory: . + paths: [api/v1/api.proto] + - directory: ../deps +plugins: + - local: protoc-gen-go + out: ../code/gen + opt: paths=source_relative +`) + oldTemplate, oldLocal, oldPaths := protoTemplate, protoLocal, protoPaths + t.Cleanup(func() { protoTemplate, protoLocal, protoPaths = oldTemplate, oldLocal, oldPaths }) + protoTemplate, protoLocal, protoPaths = "", false, []string{"api/v1/api.proto"} + require.NoError(t, generateProtoCode(t.Context(), input, input)) + _, err := os.Stat(filepath.Join(root, "code/gen/api/v1/api.pb.go")) + require.NoError(t, err, "the caller's file is generated") + for _, name := range []string{"api/v1/other.pb.go", "client/v1/client.pb.go"} { + _, err := os.Stat(filepath.Join(root, "code/gen", name)) + require.ErrorIs(t, err, os.ErrNotExist, "only the caller's selection is generated: %s", name) + } +} diff --git a/cmd/generate/proto_test.go b/cmd/generate/proto_test.go index 7e20d3c3..83d4a0b7 100644 --- a/cmd/generate/proto_test.go +++ b/cmd/generate/proto_test.go @@ -44,7 +44,13 @@ func TestProtoGenerationArgsPreserveBufInputSelection(t *testing.T) { {"v2 default", "version: v2\n", nil, []string{"generate", "/workspace/proto", "--template", "/staging/template.yaml"}}, {"empty inputs", "version: v2\ninputs: []\n", nil, []string{"generate", "/workspace/proto", "--template", "/staging/template.yaml"}}, {"declared selection", "version: v2\ninputs:\n - directory: .\n paths: [api.proto]\n - directory: ../other\n", nil, []string{"generate", "--template", "/staging/template.yaml"}}, + // One declared input keeps its input-level options beside a caller's + // --path: nothing else can be targeted by it. {"caller override", "version: v2\ninputs:\n - directory: .\n paths: [api.proto]\n", []string{"chosen.proto"}, []string{"generate", "--template", "/staging/template.yaml", "--path", "/workspace/proto/chosen.proto"}}, + {"caller selection keeps a single input's options", "version: v2\ninputs:\n - directory: .\n types: [acme.v1.Api]\n exclude_paths: [internal]\n", []string{"api.proto"}, []string{"generate", "--template", "/staging/template.yaml", "--path", "/workspace/proto/api.proto"}}, + // buf applies --path to every input: a second input that does not hold + // the caller's file would target nothing and fail the run. + {"caller selection over two inputs", "version: v2\ninputs:\n - directory: .\n paths: [api.proto]\n - directory: ../deps\n", []string{"api.proto"}, []string{"generate", "/workspace/proto", "--template", "/staging/template.yaml", "--path", "/workspace/proto/api.proto"}}, } { t.Run(tc.name, func(t *testing.T) { protoPaths = tc.paths diff --git a/cmd/publish/clients.go b/cmd/publish/clients.go index 2dcb7b95..f1e7e61a 100644 --- a/cmd/publish/clients.go +++ b/cmd/publish/clients.go @@ -17,6 +17,7 @@ import ( "github.com/Masterminds/semver" "github.com/codefly-dev/cli/cmd/common" + "github.com/codefly-dev/cli/cmd/generate" "github.com/codefly-dev/cli/pkg/cli" clicomposition "github.com/codefly-dev/cli/pkg/composition" "github.com/codefly-dev/cli/pkg/generators" @@ -465,7 +466,7 @@ func resolveClientsOutputRoot(flag string) (root string, cleanup func(), err err // Whatever shipped is immutable, so a partial failure is recorded before it // is returned and a re-run publishes only the languages still missing. func publishClientWork(ctx context.Context, run *clientsRun, item *clientWork, cfg librarystore.StoreConfig, policy librarystore.RepositoryPolicy, outputRoot string, w io.Writer) error { - entry, err := generators.BuildModuleClientEntry(ctx, run.module.Dir(), run.module.Name, run.catalog, item.plan) + entry, err := generators.BuildModuleClientEntry(ctx, run.module.Dir(), run.module.Name, run.catalog, item.plan, generate.ListProtoSources) if err != nil { return err } diff --git a/cmd/publish/clients_proto_sources_companion_test.go b/cmd/publish/clients_proto_sources_companion_test.go new file mode 100644 index 00000000..7c728fb9 --- /dev/null +++ b/cmd/publish/clients_proto_sources_companion_test.go @@ -0,0 +1,49 @@ +//go:build proto_companion_required + +package publish + +import ( + "os" + "path/filepath" + "testing" + + "github.com/codefly-dev/cli/cmd/generate" + "github.com/codefly-dev/cli/pkg/generators" + "github.com/codefly-dev/core/composition" + "github.com/stretchr/testify/require" +) + +// `generate contracts` publishes a service's proto tree verbatim beside its +// contract, buf.yaml included. Over a tree with a vendor module and excluded +// folders (module "." excludes codefly and vendor; vendor is its own module), +// publish clients names the sources as buf does: the vendored file by its +// import path, never vendor/..., and the excluded facade not at all. +func TestPublishedContractSourcesFollowBufModulesThroughPinnedCompanion(t *testing.T) { + moduleDir := t.TempDir() + contractDir := filepath.Join(moduleDir, "contracts", "api", "accounts", "connect") + contract := []byte("descriptor bytes") + for rel, content := range map[string]string{ + "contract.binpb": string(contract), + "proto/buf.yaml": "version: v2\nmodules:\n - path: .\n excludes: [codefly, vendor]\n - path: vendor\n", + "proto/accounts/v1/accounts.proto": "syntax = \"proto3\";\npackage accounts.v1;\nimport \"codefly/base/v0/base.proto\";\nmessage Account { codefly.base.v0.Ref ref = 1; }\n", + "proto/codefly/api.proto": "syntax = \"proto3\";\npackage codefly.facade;\nimport \"accounts/v1/accounts.proto\";\nmessage Facade { accounts.v1.Account account = 1; }\n", + "proto/vendor/codefly/base/v0/base.proto": "syntax = \"proto3\";\npackage codefly.base.v0;\nmessage Ref { string id = 1; }\n", + } { + path := filepath.Join(contractDir, filepath.FromSlash(rel)) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o755)) + require.NoError(t, os.WriteFile(path, []byte(content), 0o600)) + } + plan := &generators.ModuleClientPlan{Endpoint: composition.APIContractEndpoint{ + Service: "accounts", Endpoint: "connect", Kind: composition.APIContractKindProtobuf, + Path: "contracts/api/accounts/connect/contract.binpb", Digest: composition.APIContractDigest(contract), + }} + catalog := &composition.APIContractCatalog{Package: "codefly/saas-starter", Version: "0.1.0"} + + entry, err := generators.BuildModuleClientEntry(t.Context(), moduleDir, "saas-starter", catalog, plan, generate.ListProtoSources) + require.NoError(t, err) + var names []string + for _, source := range entry.ProtoSources { + names = append(names, source.Path) + } + require.Equal(t, []string{"accounts/v1/accounts.proto", "codefly/base/v0/base.proto"}, names) +} diff --git a/pkg/generators/module_clients.go b/pkg/generators/module_clients.go index a17369b5..034b6cb9 100644 --- a/pkg/generators/module_clients.go +++ b/pkg/generators/module_clients.go @@ -5,7 +5,6 @@ import ( "errors" "fmt" "io" - "io/fs" "os" "path/filepath" "sort" @@ -172,11 +171,18 @@ func PlanModuleClients(moduleName string, catalog *composition.APIContractCatalo return plans, nil } +// ProtoSourceLister names a proto tree's source set as buf compiles it. +// `codefly generate` owns the one implementation (generate.ListProtoSources), +// which runs buf in the pinned proto companion. +type ProtoSourceLister func(ctx context.Context, protoDir string) ([]coreproto.Source, error) + // BuildModuleClientEntry reads exactly the committed package contract that is // being published. Callers validate the package manifest, catalog and artifact // digests before invoking it, so generated clients cannot diverge from the -// module package under the same version. -func BuildModuleClientEntry(ctx context.Context, moduleDir, moduleName string, catalog *composition.APIContractCatalog, plan *ModuleClientPlan) (*ContractEntry, error) { +// module package under the same version. A protobuf contract's sources are +// what listSources names over the proto tree published beside it, so buf.yaml +// modules and excludes decide them exactly as they did the contract. +func BuildModuleClientEntry(ctx context.Context, moduleDir, moduleName string, catalog *composition.APIContractCatalog, plan *ModuleClientPlan, listSources ProtoSourceLister) (*ContractEntry, error) { contractBytes, err := ReadCatalogContractBytes(moduleDir, &plan.Endpoint) if err != nil { return nil, fmt.Errorf("read contract for %s/%s: %w", plan.Endpoint.Service, plan.Endpoint.Endpoint, err) @@ -184,7 +190,7 @@ func BuildModuleClientEntry(ctx context.Context, moduleDir, moduleName string, c var protoSources []coreproto.Source if plan.Endpoint.Kind == composition.APIContractKindProtobuf { protoDir := filepath.Join(filepath.Dir(filepath.Join(moduleDir, filepath.FromSlash(plan.Endpoint.Path))), "proto") - protoSources, err = collectPublishedProtoSources(ctx, protoDir) + protoSources, err = listSources(ctx, protoDir) if err != nil { return nil, fmt.Errorf("read proto sources for %s/%s: %w", plan.Endpoint.Service, plan.Endpoint.Endpoint, err) } @@ -201,39 +207,6 @@ func BuildModuleClientEntry(ctx context.Context, moduleDir, moduleName string, c }, nil } -func collectPublishedProtoSources(ctx context.Context, protoDir string) ([]coreproto.Source, error) { - var sources []coreproto.Source - err := filepath.WalkDir(protoDir, func(path string, entry fs.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr - } - if err := ctx.Err(); err != nil { - return err - } - if entry.IsDir() || filepath.Ext(path) != ".proto" { - return nil - } - rel, err := filepath.Rel(protoDir, path) - if err != nil { - return err - } - content, err := os.ReadFile(path) //nolint:gosec // walked beneath the validated contract directory. - if err != nil { - return err - } - sources = append(sources, coreproto.Source{Path: filepath.ToSlash(rel), Content: content}) - return nil - }) - if err != nil { - return nil, err - } - if len(sources) == 0 { - return nil, fmt.Errorf("no .proto files under %s", protoDir) - } - sort.Slice(sources, func(i, j int) bool { return sources[i].Path < sources[j].Path }) - return sources, nil -} - func containsLanguage(list []languages.Language, lang languages.Language) bool { for _, candidate := range list { if candidate == lang { diff --git a/pkg/generators/module_clients_test.go b/pkg/generators/module_clients_test.go index 4ca3ef91..5545e40a 100644 --- a/pkg/generators/module_clients_test.go +++ b/pkg/generators/module_clients_test.go @@ -1,10 +1,12 @@ package generators import ( + "context" "os" "path/filepath" "testing" + coreproto "github.com/codefly-dev/core/companions/proto" "github.com/codefly-dev/core/composition" "github.com/codefly-dev/core/languages" "github.com/stretchr/testify/require" @@ -147,9 +149,8 @@ func TestBuildModuleClientEntryUsesCommittedPackageArtifacts(t *testing.T) { dir := t.TempDir() contract := []byte("descriptor bytes") contractPath := filepath.Join(dir, "contracts", "api", "accounts", "connect", "contract.binpb") - require.NoError(t, os.MkdirAll(filepath.Join(filepath.Dir(contractPath), "proto", "accounts", "v1"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Dir(contractPath), 0o755)) require.NoError(t, os.WriteFile(contractPath, contract, 0o644)) - require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(contractPath), "proto", "accounts", "v1", "accounts.proto"), []byte("syntax = \"proto3\";"), 0o644)) catalog := &composition.APIContractCatalog{Package: "codefly/saas-starter", Version: "0.1.0"} plan := &ModuleClientPlan{ Endpoint: composition.APIContractEndpoint{ @@ -158,9 +159,17 @@ func TestBuildModuleClientEntryUsesCommittedPackageArtifacts(t *testing.T) { }, Services: []string{"AccountsService"}, } - entry, err := BuildModuleClientEntry(t.Context(), dir, "saas-starter", catalog, plan) + // The sources are what the lister names over the proto tree published + // beside the contract; nothing here walks that tree itself. + listed := []coreproto.Source{{Path: "accounts/v1/accounts.proto", Content: []byte("syntax = \"proto3\";")}} + var listedDir string + entry, err := BuildModuleClientEntry(t.Context(), dir, "saas-starter", catalog, plan, func(_ context.Context, protoDir string) ([]coreproto.Source, error) { + listedDir = protoDir + return listed, nil + }) require.NoError(t, err) + require.Equal(t, filepath.Join(filepath.Dir(contractPath), "proto"), listedDir) require.Equal(t, contract, entry.ContractBytes) - require.Equal(t, "accounts/v1/accounts.proto", entry.ProtoSources[0].Path) + require.Equal(t, listed, entry.ProtoSources) require.Equal(t, []string{"AccountsService"}, entry.Services) }