From c2d2ec700081d9a63227343d9cfbde9491701ca6 Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Thu, 21 Jul 2022 17:42:18 +0200 Subject: [PATCH 1/8] put step: skeleton of the new executable without ofcourse This is the absolute minimum to have a skeleton executable for /opt/resource/out and passing tests. We will build upon this commit. PCI-2587 --- Dockerfile | 6 +- Taskfile.yml | 2 +- cmd/cogito/main.go | 2 + cmd/cogito/main_test.go | 12 ++++ cmd/out/main.go | 10 --- cogito/get_test.go | 1 - cogito/protocol.go | 23 +++++++ cogito/put.go | 69 +++++++++++++++++++++ cogito/put_test.go | 124 ++++++++++++++++++++++++++++++++++++++ resource/resource_test.go | 106 -------------------------------- 10 files changed, 234 insertions(+), 121 deletions(-) delete mode 100644 cmd/out/main.go create mode 100644 cogito/put.go create mode 100644 cogito/put_test.go diff --git a/Dockerfile b/Dockerfile index 3e999e0d..73c736bc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -22,8 +22,7 @@ COPY . . RUN go test ./... && \ go install \ -ldflags "-w -X 'github.com/Pix4D/cogito/resource.buildinfo=$BUILD_INFO'" \ - ./cmd/cogito \ - ./cmd/out + ./cmd/cogito # # The final image @@ -38,4 +37,5 @@ RUN mkdir -p /opt/resource COPY --from=builder /root/go/bin/* /opt/resource/ RUN ln -s /opt/resource/cogito /opt/resource/check && \ - ln -s /opt/resource/cogito /opt/resource/in + ln -s /opt/resource/cogito /opt/resource/in && \ + ln -s /opt/resource/cogito /opt/resource/out diff --git a/Taskfile.yml b/Taskfile.yml index 6e9bb2d2..a3f04f55 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -89,10 +89,10 @@ tasks: dir: bin cmds: - go build -ldflags "{{.LDFLAGS}}" ../cmd/cogito - - go build -ldflags "{{.LDFLAGS}}" ../cmd/out - go build -ldflags "{{.LDFLAGS}}" ../cmd/hello - ln -sf cogito check - ln -sf cogito in + - ln -sf cogito out build:copydir: desc: Build copydir (development helper, normally not needed). diff --git a/cmd/cogito/main.go b/cmd/cogito/main.go index 5611b6d7..0ccc01c3 100644 --- a/cmd/cogito/main.go +++ b/cmd/cogito/main.go @@ -38,6 +38,8 @@ func run(in io.Reader, out io.Writer, logOut io.Writer, args []string) error { return cogito.Check(log, in, out, args[1:]) case "in": return cogito.Get(log, in, out, args[1:]) + case "out": + return cogito.Put(log, in, out, args[1:]) default: return fmt.Errorf( "cogito: unexpected invocation as '%s'; want: one of 'check', 'in', 'out'; (command-line: %s)", diff --git a/cmd/cogito/main_test.go b/cmd/cogito/main_test.go index 6b1b5576..bee67270 100644 --- a/cmd/cogito/main_test.go +++ b/cmd/cogito/main_test.go @@ -50,6 +50,18 @@ func TestRunSmokeSuccess(t *testing.T) { "access_token": "the-secret" }, "version": {"ref": "pizza"} +}`, + }, + { + name: "put", + args: []string{"out", "dummy-dir"}, + in: ` +{ + "source": { + "owner": "the-owner", + "repo": "the-repo", + "access_token": "the-secret" + } }`, }, } diff --git a/cmd/out/main.go b/cmd/out/main.go deleted file mode 100644 index fedf6d18..00000000 --- a/cmd/out/main.go +++ /dev/null @@ -1,10 +0,0 @@ -package main - -import ( - "github.com/Pix4D/cogito/resource" - "github.com/cloudboss/ofcourse/ofcourse" -) - -func main() { - ofcourse.Out(resource.New()) -} diff --git a/cogito/get_test.go b/cogito/get_test.go index 662bd306..4ebb1048 100644 --- a/cogito/get_test.go +++ b/cogito/get_test.go @@ -127,7 +127,6 @@ func TestGetFailure(t *testing.T) { }, { name: "system read error", - source: cogito.Source{}, reader: iotest.ErrReader(errors.New("test read error")), writer: io.Discard, wantErr: "get: parsing JSON from stdin: test read error", diff --git a/cogito/protocol.go b/cogito/protocol.go index 365a4db3..0c3a3a98 100644 --- a/cogito/protocol.go +++ b/cogito/protocol.go @@ -35,6 +35,17 @@ type GetInput struct { Env Environment } +// PutInput is the JSON object passed to the stdin of the "out" executable plus +// build metadata (environment variables). +// +// See https://concourse-ci.org/implementing-resource-types.html#resource-out +// +type PutInput struct { + Source Source `json:"source"` + Params PutParams `json:"params"` + Env Environment +} + // Source is the "source:" block in a pipeline "resources:" block for the Cogito resource. type Source struct { // @@ -164,6 +175,18 @@ type Output struct { // declare it at all. // type GetParams struct{} +// PutParams is the "params:" block in a pipeline put step for the Cogito resource. +type PutParams struct { + // + // Mandatory + // + State string `json:"state"` + // + // Optional + // + Context string `json:"context"` +} + // Environment represents the environment variables made available to the program. // Depending on the type of build and on the step, only some variables could be set. // See https://concourse-ci.org/implementing-resource-types.html#resource-metadata diff --git a/cogito/put.go b/cogito/put.go new file mode 100644 index 00000000..911fd4e1 --- /dev/null +++ b/cogito/put.go @@ -0,0 +1,69 @@ +package cogito + +import ( + "encoding/json" + "fmt" + "io" + + "github.com/hashicorp/go-hclog" +) + +// Put implements the "put" step (the "out" executable). +// +// From https://concourse-ci.org/implementing-resource-types.html#resource-out: +// +// The out script is passed a path to the directory containing the build's full set of +// sources as command line argument $1, and is given on stdin the configured params and +// the resource's source configuration. +// +// The script must emit the resulting version of the resource. +// +// Additionally, the script may emit metadata as a list of key-value pairs. This data is +// intended for public consumption and will make it upstream, intended to be shown on the +// build's page. +func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { + var pi PutInput + dec := json.NewDecoder(in) + dec.DisallowUnknownFields() + if err := dec.Decode(&pi); err != nil { + return fmt.Errorf("put: parsing JSON from stdin: %s", err) + } + pi.Env.Fill() + + if err := pi.Source.ValidateLog(); err != nil { + return fmt.Errorf("put: %s", err) + } + log = log.Named("put") + log.SetLevel(hclog.LevelFromString(pi.Source.LogLevel)) + + log.Debug("started", + "source", pi.Source, + "params", pi.Params, + "environment", pi.Env, + "args", args) + + if err := pi.Source.Validate(); err != nil { + return fmt.Errorf("put: %s", err) + } + + // args[0] contains the path to a Concourse volume containing all the resource + // "put inputs". + if len(args) == 0 { + return fmt.Errorf("put: missing input directory from arguments") + } + inputDir := args[0] + log.Debug("", "input-directory", inputDir) + + // Following the protocol for put, we return a dummy version and metadata. + output := Output{ + // TODO Version: constant dummy + // TODO Metadata + } + enc := json.NewEncoder(out) + if err := enc.Encode(output); err != nil { + return fmt.Errorf("put: %s", err) + } + + log.Debug("success", "output", output) + return nil +} diff --git a/cogito/put_test.go b/cogito/put_test.go new file mode 100644 index 00000000..72ec2759 --- /dev/null +++ b/cogito/put_test.go @@ -0,0 +1,124 @@ +package cogito_test + +import ( + "bytes" + "errors" + "io" + "testing" + "testing/iotest" + + "github.com/Pix4D/cogito/cogito" + "github.com/hashicorp/go-hclog" + "gotest.tools/v3/assert" +) + +func TestPutSuccess(t *testing.T) { + type testCase struct { + name string + in cogito.PutInput + } + + test := func(t *testing.T, tc testCase) { + in := bytes.NewReader(toJSON(t, tc.in)) + var out bytes.Buffer + log := hclog.NewNullLogger() + + err := cogito.Put(log, in, &out, []string{"dummy-dir"}) + + assert.NilError(t, err) + } + + baseSource := cogito.Source{ + Owner: "the-owner", + Repo: "the-repo", + AccessToken: "the-token", + } + + testCases := []testCase{ + { + name: "minimal smoke", + in: cogito.PutInput{ + Source: baseSource, + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + test(t, tc) + }) + } +} + +func TestPutFailure(t *testing.T) { + type testCase struct { + name string + args []string + source cogito.Source // will be embedded in cogito.PutInput + reader io.Reader // if set, will override fields source above. + writer io.Writer + wantErr string + } + + test := func(t *testing.T, tc testCase) { + assert.Assert(t, tc.wantErr != "") + in := tc.reader + if in == nil { + in = bytes.NewReader(toJSON(t, + cogito.PutInput{ + Source: tc.source, + })) + } + log := hclog.NewNullLogger() + + err := cogito.Put(log, in, tc.writer, tc.args) + + assert.Error(t, err, tc.wantErr) + } + + baseSource := cogito.Source{ + Owner: "the-owner", + Repo: "the-repo", + AccessToken: "the-token", + } + + testCases := []testCase{ + { + name: "user validation failure: missing keys", + source: cogito.Source{}, + writer: io.Discard, + wantErr: "put: source: missing keys: owner, repo, access_token", + }, + { + name: "user validation failure: log_level", + source: mergeStructs(baseSource, cogito.Source{LogLevel: "pippo"}), + writer: io.Discard, + wantErr: "put: source: invalid log_level: pippo", + }, + { + name: "concourse validation failure: missing input directory", + source: baseSource, + writer: io.Discard, + wantErr: "put: missing input directory from arguments", + }, + { + name: "system write error", + args: []string{"dummy-dir"}, + source: baseSource, + writer: &failingWriter{}, + wantErr: "put: test write error", + }, + { + name: "system read error", + reader: iotest.ErrReader(errors.New("test read error")), + writer: io.Discard, + wantErr: "put: parsing JSON from stdin: test read error", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + test(t, tc) + }) + } +} diff --git a/resource/resource_test.go b/resource/resource_test.go index 645a8352..094e1eb9 100644 --- a/resource/resource_test.go +++ b/resource/resource_test.go @@ -757,109 +757,3 @@ func TestParseGitPseudoURLFailure(t *testing.T) { }) } } - -func TestValidateSourceSuccess(t *testing.T) { - testCases := []struct { - name string - source oc.Source - }{ - { - name: "all mandatory keys, no optional", - source: oc.Source{ - accessTokenKey: "dummy-token", - ownerKey: "dummy-owner", - repoKey: "dummy-repo", - }, - }, - { - name: "all mandatory and optional keys", - source: oc.Source{ - accessTokenKey: "dummy-token", - ownerKey: "dummy-owner", - repoKey: "dummy-repo", - // - logLevelKey: "dummy", - logUrlKey: "dummy", - contextPrefixKey: "dummy", - // - gchatWebhookKey: "dummy", - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - - err := validateSource(tc.source) - - if err != nil { - t.Fatalf("\nhave: %s\nwant: ", err) - } - }) - } -} - -func TestValidateSourceFailure(t *testing.T) { - testCases := []struct { - name string - source oc.Source - wantErr string - }{ - { - name: "zero keys", - source: oc.Source{}, - wantErr: "missing source keys: [access_token owner repo]", - }, - { - name: "missing mandatory keys", - source: oc.Source{ - repoKey: "dummy-repo", - }, - wantErr: "missing source keys: [access_token owner]", - }, - { - name: "all mandatory keys, one unknown key", - source: oc.Source{ - accessTokenKey: "dummy-token", - ownerKey: "dummy-owner", - repoKey: "dummy-repo", - - "pizza": "napoli", - }, - wantErr: "unknown source keys: [pizza]", - }, - { - name: "one missing mandatory key, one unknown key", - source: oc.Source{ - ownerKey: "dummy-owner", - repoKey: "dummy-repo", - - "pizza": "napoli", - }, - wantErr: "missing source keys: [access_token]; unknown source keys: [pizza]", - }, - { - name: "wrong type is reported as missing (better than crashing)", - source: oc.Source{ - accessTokenKey: "dummy-token", - ownerKey: 3, - repoKey: "dummy-repo", - }, - wantErr: "missing source keys: [owner]", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - - err := validateSource(tc.source) - - if err == nil { - t.Fatalf("\nhave: \nwant: %s", tc.wantErr) - } - if diff := cmp.Diff(tc.wantErr, err.Error()); diff != "" { - t.Errorf("error message mismatch: (-want +have):\n%s", diff) - } - }) - } -} From d6b94bad1da4eb9fae81f4c7568e2da859e32878 Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Fri, 22 Jul 2022 14:04:40 +0200 Subject: [PATCH 2/8] disable the building and testing of the "resource" pkg This is needed to be able to run build and tests successfully. At the end of this branch, all the code in the "resource" pkg will be migrated to the "cogito" pkg, thus all tests and functionalities will be back. PCI-2587 --- Dockerfile | 4 ++-- Taskfile.yml | 8 +++++--- resource/resource.go | 22 ---------------------- 3 files changed, 7 insertions(+), 27 deletions(-) diff --git a/Dockerfile b/Dockerfile index 73c736bc..34e9e1f6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -19,8 +19,8 @@ RUN go mod download # COPY . . -RUN go test ./... && \ - go install \ +RUN go test ./cogito ./cmd/cogito +RUN go install \ -ldflags "-w -X 'github.com/Pix4D/cogito/resource.buildinfo=$BUILD_INFO'" \ ./cmd/cogito diff --git a/Taskfile.yml b/Taskfile.yml index a3f04f55..a33d0125 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -63,12 +63,14 @@ tasks: desc: Run the unit tests. cmds: # One day I will understand how to use -coverpkg=./... :-( - - go test -count=1 -short -coverprofile=coverage.out ./... + # FIXME - go test -count=1 -short -coverprofile=coverage.out ./... + - go test -count=1 -short -coverprofile=coverage.out ./cogito ./cmd/cogito test:all: desc: Run all the tests (unit + integration). Use this target to get total coverage. cmds: - - go test -count=1 -coverprofile=coverage.out ./... + # FIXME - go test -count=1 -coverprofile=coverage.out ./... + - go test -count=1 -short -coverprofile=coverage.out ./cogito ./cmd/cogito env: *test-env test:smoke: @@ -89,7 +91,7 @@ tasks: dir: bin cmds: - go build -ldflags "{{.LDFLAGS}}" ../cmd/cogito - - go build -ldflags "{{.LDFLAGS}}" ../cmd/hello + # FIXME - go build -ldflags "{{.LDFLAGS}}" ../cmd/hello - ln -sf cogito check - ln -sf cogito in - ln -sf cogito out diff --git a/resource/resource.go b/resource/resource.go index 613f58a6..fe077c3d 100644 --- a/resource/resource.go +++ b/resource/resource.go @@ -114,28 +114,6 @@ func NewWith(githubAPI string) *Resource { } } -// Check satisfies ofcourse.Resource.Check(), corresponding to the /opt/resource/check command. -func (r *Resource) Check( - source oc.Source, - version oc.Version, - env oc.Environment, - log *oc.Logger, -) ([]oc.Version, error) { - return nil, errors.New("old 'check', kept to satisfy the ofcourse.Resource interface") -} - -// In satisfies ofcourse.Resource.In(), corresponding to the /opt/resource/in command. -func (r *Resource) In( - outputDir string, - source oc.Source, - params oc.Params, - version oc.Version, - env oc.Environment, - log *oc.Logger, -) (oc.Version, oc.Metadata, error) { - return nil, nil, errors.New("old 'get', kept to satisfy the ofcourse.Resource interface") -} - // Out satisfies ofcourse.Resource.Out(), corresponding to the /opt/resource/out command. func (r *Resource) Out( inputDir string, // All the resource "put inputs" are below this directory. From ac9e15b8b7324ec1275be89541f07bfa5a1843f9 Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Fri, 22 Jul 2022 14:08:59 +0200 Subject: [PATCH 3/8] move buildinfo from cogito/resource to cogito While there, finally found an easy way to test that the executable, when built with Task, contains the build info. PCI-2587 --- Dockerfile | 2 +- Taskfile.yml | 27 ++++++++++++++++++++++++++- cmd/cogito/main.go | 3 +-- cogito/buildinfo.go | 11 +++++++++++ resource/resource.go | 10 ---------- 5 files changed, 39 insertions(+), 14 deletions(-) create mode 100644 cogito/buildinfo.go diff --git a/Dockerfile b/Dockerfile index 34e9e1f6..c36ab69a 100644 --- a/Dockerfile +++ b/Dockerfile @@ -21,7 +21,7 @@ COPY . . RUN go test ./cogito ./cmd/cogito RUN go install \ - -ldflags "-w -X 'github.com/Pix4D/cogito/resource.buildinfo=$BUILD_INFO'" \ + -ldflags "-w -X 'github.com/Pix4D/cogito/cogito.buildinfo=$BUILD_INFO'" \ ./cmd/cogito # diff --git a/Taskfile.yml b/Taskfile.yml index a33d0125..ebfafa73 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -17,7 +17,7 @@ vars: DOCKER_BASE_NAME: "{{.DOCKER_ORG}}/{{.DOCKER_IMAGE}}" DOCKER_FULL_NAME: "{{.DOCKER_BASE_NAME}}:{{.DOCKER_TAG}}" BUILD_INFO: "Tag: {{.DOCKER_TAG}}, commit: {{.COMMIT}}, build date: {{.DATE}}" - LDFLAGS: -w -X '{{.REPO}}/resource.buildinfo={{.BUILD_INFO}}' + LDFLAGS: -w -X '{{.REPO}}/cogito.buildinfo={{.BUILD_INFO}}' # SMOKE_INPUT: > { @@ -76,10 +76,35 @@ tasks: test:smoke: desc: Simple smoke test of the local executables. cmds: + - task: test:buildinfo - echo '{{.SMOKE_INPUT}}' | ./bin/check - echo - echo '{{.SMOKE_INPUT}}' | ./bin/in dummy-dir - echo + - echo '{{.PUT_INPUT}}' | ./bin/out dummy-dir + - echo + vars: + PUT_INPUT: > + { + "source": {"owner": "foo", "repo": "bar", "access_token": "123", "log_level": "debug"}, + "params": {"state": "success"} + } + + test:buildinfo: + desc: Verify that the executable contains build information + # cogito: This is the Cogito GitHub status resource. unknown + # cogito: This is the Cogito GitHub status resource. Tag: buildinfo, commit: e9b36d0814, build date: 2022-07-26 + cmds: + # "unknown" is the default value, printed when built without linker flags. + - 'echo {{.OUTPUT}} | grep -v unknown' + - 'echo {{.OUTPUT}} | grep Tag:' + - 'echo {{.OUTPUT}} | grep commit:' + vars: + INPUT: '{"source": {"owner": "foo", "repo": "bar", "access_token": "123"}}' + OUTPUT: + # We only want to capture stderr, because the Cogito resource protocol uses + # stderr for logging. + sh: echo '{{.INPUT}}' | ./bin/check 2>&1 1>/dev/null browser: desc: "Show code coverage in browser (usage: task test: browser)" diff --git a/cmd/cogito/main.go b/cmd/cogito/main.go index 0ccc01c3..2f550c64 100644 --- a/cmd/cogito/main.go +++ b/cmd/cogito/main.go @@ -9,7 +9,6 @@ import ( "path" "github.com/Pix4D/cogito/cogito" - "github.com/Pix4D/cogito/resource" "github.com/hashicorp/go-hclog" ) @@ -30,7 +29,7 @@ func run(in io.Reader, out io.Writer, logOut io.Writer, args []string) error { Output: logOut, DisableTime: true, }) - log.Info(resource.BuildInfo()) + log.Info(cogito.BuildInfo()) command := path.Base(args[0]) switch command { diff --git a/cogito/buildinfo.go b/cogito/buildinfo.go new file mode 100644 index 00000000..6d39bb29 --- /dev/null +++ b/cogito/buildinfo.go @@ -0,0 +1,11 @@ +package cogito + +// Baked in at build time with the linker. See the Taskfile and the Dockerfile. +var buildinfo = "unknown" + +// BuildInfo returns human-readable build information (tag, git commit, date, ...). +// This is useful to understand in the Concourse UI and logs which resource it is, since log +// output in Concourse doesn't mention the name of the resource (or task) generating it. +func BuildInfo() string { + return "This is the Cogito GitHub status resource. " + buildinfo +} diff --git a/resource/resource.go b/resource/resource.go index fe077c3d..a071f24b 100644 --- a/resource/resource.go +++ b/resource/resource.go @@ -19,9 +19,6 @@ import ( oc "github.com/cloudboss/ofcourse/ofcourse" ) -// Baked in at build time with the linker. See the Taskfile and the Dockerfile. -var buildinfo = "unknown" - const ( accessTokenKey = "access_token" gchatWebhookKey = "gchat_webhook" @@ -83,13 +80,6 @@ var ( statesToNotifyChat = []string{abortState, errorState, failureState} ) -// BuildInfo returns human-readable build information (tag, git commit, date, ...). -// This is useful to understand in the Concourse UI and logs which resource it is, since log -// output in Concourse doesn't mention the name of the resource (or task) generating it. -func BuildInfo() string { - return "This is the Cogito GitHub status resource. " + buildinfo -} - // Resource satisfies the ofcourse.Resource interface. type Resource struct { githubAPI string From 06229e0054e3eb687ed7b7aac29b4f108861c4b1 Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Fri, 22 Jul 2022 16:05:27 +0200 Subject: [PATCH 4/8] put step: add put params and output metadata PCI-2587 --- cmd/cogito/main.go | 2 +- cmd/cogito/main_test.go | 84 ++++++++++---------- cogito/check.go | 2 +- cogito/get.go | 2 +- cogito/get_test.go | 2 +- cogito/protocol.go | 45 ++++++++++- cogito/put.go | 18 +++-- cogito/put_test.go | 164 +++++++++++++++++++++++++++++++--------- resource/resource.go | 6 -- 9 files changed, 225 insertions(+), 100 deletions(-) diff --git a/cmd/cogito/main.go b/cmd/cogito/main.go index 2f550c64..16e4b6e8 100644 --- a/cmd/cogito/main.go +++ b/cmd/cogito/main.go @@ -18,7 +18,7 @@ func main() { // - stderr for logging // See: https://concourse-ci.org/implementing-resource-types.html if err := run(os.Stdin, os.Stdout, os.Stderr, os.Args); err != nil { - fmt.Fprintln(os.Stderr, err) + fmt.Fprintf(os.Stderr, "Error: %s\n", err) os.Exit(1) } } diff --git a/cmd/cogito/main_test.go b/cmd/cogito/main_test.go index bee67270..be5443e0 100644 --- a/cmd/cogito/main_test.go +++ b/cmd/cogito/main_test.go @@ -9,68 +9,60 @@ import ( "gotest.tools/v3/assert" ) -func TestRunSmokeSuccess(t *testing.T) { - type testCase struct { - name string - args []string - in string - } - - test := func(t *testing.T, tc testCase) { - in := strings.NewReader(tc.in) - var out bytes.Buffer - var logOut bytes.Buffer - - err := run(in, &out, &logOut, tc.args) - - assert.NilError(t, err, "\nout: %s\nlogOut: %s", out.String(), logOut.String()) - } - - testCases := []testCase{ - { - name: "check", - args: []string{"check"}, - in: ` +func TestRunCheckSmokeSuccess(t *testing.T) { + in := strings.NewReader(` { "source": { "owner": "the-owner", "repo": "the-repo", - "access_token": "the-secret" + "access_token": "the-secret", + "log_level": "debug" } -}`, - }, - { - name: "get", - args: []string{"in", "dummy-dir"}, - in: ` +}`) + var out bytes.Buffer + var logOut bytes.Buffer + + err := run(in, &out, &logOut, []string{"check"}) + + assert.NilError(t, err, "\nout: %s\nlogOut: %s", out.String(), logOut.String()) +} + +func TestRunGetSmokeSuccess(t *testing.T) { + in := strings.NewReader(` { "source": { "owner": "the-owner", "repo": "the-repo", - "access_token": "the-secret" + "access_token": "the-secret", + "log_level": "debug" }, "version": {"ref": "pizza"} -}`, - }, - { - name: "put", - args: []string{"out", "dummy-dir"}, - in: ` +}`) + var out bytes.Buffer + var logOut bytes.Buffer + + err := run(in, &out, &logOut, []string{"in", "dummy-dir"}) + + assert.NilError(t, err, "\nout: %s\nlogOut: %s", out.String(), logOut.String()) +} + +func TestRunPutSmokeSuccess(t *testing.T) { + in := strings.NewReader(` { "source": { "owner": "the-owner", "repo": "the-repo", - "access_token": "the-secret" - } -}`, - }, - } + "access_token": "the-secret", + "log_level": "debug" + }, + "params": {"state": "pending"} +}`) + var out bytes.Buffer + var logOut bytes.Buffer - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - test(t, tc) - }) - } + err := run(in, &out, &logOut, []string{"out", "dummy-dir"}) + + assert.NilError(t, err, "\nout: %s\nlogOut: %s", out.String(), logOut.String()) } func TestRunSmokeFailure(t *testing.T) { diff --git a/cogito/check.go b/cogito/check.go index 930f3aa8..b0f1dc28 100644 --- a/cogito/check.go +++ b/cogito/check.go @@ -55,7 +55,7 @@ func Check(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { // could not be empty. Now (2022-07) it seems that it could indeed be empty. // For the time being we keep it as-is because this maintains the previous behavior. // This will be investigated by PCI-2617. - versions := []Version{{Ref: "dummy"}} + versions := []Version{DummyVersion} enc := json.NewEncoder(out) if err := enc.Encode(versions); err != nil { return fmt.Errorf("check: %s", err) diff --git a/cogito/get.go b/cogito/get.go index 47be32fa..53c714a6 100644 --- a/cogito/get.go +++ b/cogito/get.go @@ -58,7 +58,7 @@ func Get(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { // and put there the requested version of the resource. // In this resource we do nothing, but we still check for protocol conformance. if len(args) == 0 { - return fmt.Errorf("get: missing output directory from arguments") + return fmt.Errorf("get: arguments: missing output directory") } log.Debug("", "output-directory", args[0]) diff --git a/cogito/get_test.go b/cogito/get_test.go index 4ebb1048..750aa9aa 100644 --- a/cogito/get_test.go +++ b/cogito/get_test.go @@ -115,7 +115,7 @@ func TestGetFailure(t *testing.T) { source: baseSource, version: cogito.Version{Ref: "dummy"}, writer: io.Discard, - wantErr: "get: missing output directory from arguments", + wantErr: "get: arguments: missing output directory", }, { name: "system write error", diff --git a/cogito/protocol.go b/cogito/protocol.go index 0c3a3a98..c2f93814 100644 --- a/cogito/protocol.go +++ b/cogito/protocol.go @@ -9,6 +9,10 @@ import ( "strings" ) +// DummyVersion is the version always returned by the Cogito resource. +// DO NOT REASSIGN! +var DummyVersion = Version{Ref: "dummy"} + // CheckInput is the JSON object passed to the stdin of the "check" executable plus // build metadata (environment variables). // @@ -164,9 +168,18 @@ func (ver Version) String() string { // Output is the JSON object emitted by the get and put step. type Output struct { - Version Version `json:"version"` - // the Cogito resource doesn't use the optional "metadata" field. - // Metadata Metadata `json:"metadata"` + Version Version `json:"version"` + Metadata []Metadata `json:"metadata"` +} + +// Metadata is an element of a list of indirect k/v pairs, part of the Concourse protocol. +// +// Note that Concourse confusingly uses the term "metadata" for two completely different +// concepts: (1) the environment variables made available from Concourse to the check, get +// and put steps and (2) the metadata k/v map outputted by the get and put steps. +type Metadata struct { + Name string `json:"name"` + Value string `json:"value"` } // GetParams is the "params:" block in a pipeline get step for the Cogito resource. @@ -175,12 +188,36 @@ type Output struct { // declare it at all. // type GetParams struct{} +// BuildState is a pseudo-enum representing the valid values of PutParams.State +type BuildState string + +const ( + // NOTE: this list must be kept in sync with method Validate(). + + StateAbort BuildState = "abort" + StateError = "error" + StateFailure = "failure" + StatePending = "pending" + StateSuccess = "success" +) + +const KeyState = "state" + +// Validate checks whether the build state, parsed from JSON, is valid. +func (bs BuildState) Validate() error { + switch bs { + case StateAbort, StateError, StateFailure, StatePending, StateSuccess: + return nil + } + return fmt.Errorf("invalid build state: %s", bs) +} + // PutParams is the "params:" block in a pipeline put step for the Cogito resource. type PutParams struct { // // Mandatory // - State string `json:"state"` + State BuildState `json:"state"` // // Optional // diff --git a/cogito/put.go b/cogito/put.go index 911fd4e1..9c2d7b34 100644 --- a/cogito/put.go +++ b/cogito/put.go @@ -46,18 +46,24 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { return fmt.Errorf("put: %s", err) } - // args[0] contains the path to a Concourse volume containing all the resource - // "put inputs". + // args[0] contains the path to a directory containing all the "put inputs". if len(args) == 0 { - return fmt.Errorf("put: missing input directory from arguments") + return fmt.Errorf("put: arguments: missing input directory") } inputDir := args[0] log.Debug("", "input-directory", inputDir) - // Following the protocol for put, we return a dummy version and metadata. + buildState := pi.Params.State + if err := buildState.Validate(); err != nil { + return fmt.Errorf("put: params: %s", err) + } + log.Debug("", "state", buildState) + + // Following the protocol for put, we return the version and metadata. + // For Cogito, the metadata contains the Concourse build state. output := Output{ - // TODO Version: constant dummy - // TODO Metadata + Version: DummyVersion, + Metadata: []Metadata{{Name: KeyState, Value: string(buildState)}}, } enc := json.NewEncoder(out) if err := enc.Encode(output); err != nil { diff --git a/cogito/put_test.go b/cogito/put_test.go index 72ec2759..2d2bd51a 100644 --- a/cogito/put_test.go +++ b/cogito/put_test.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "io" + "strings" "testing" "testing/iotest" @@ -14,8 +15,9 @@ import ( func TestPutSuccess(t *testing.T) { type testCase struct { - name string - in cogito.PutInput + name string + in cogito.PutInput + wantOut cogito.Output } test := func(t *testing.T, tc testCase) { @@ -26,6 +28,9 @@ func TestPutSuccess(t *testing.T) { err := cogito.Put(log, in, &out, []string{"dummy-dir"}) assert.NilError(t, err) + var have cogito.Output + fromJSON(t, out.Bytes(), &have) + assert.DeepEqual(t, have, tc.wantOut) } baseSource := cogito.Source{ @@ -34,11 +39,18 @@ func TestPutSuccess(t *testing.T) { AccessToken: "the-token", } + baseParam := cogito.PutParams{State: cogito.StatePending} + testCases := []testCase{ { - name: "minimal smoke", + name: "returns correct version and metadata", in: cogito.PutInput{ Source: baseSource, + Params: baseParam, + }, + wantOut: cogito.Output{ + Version: cogito.DummyVersion, + Metadata: []cogito.Metadata{{Name: cogito.KeyState, Value: cogito.StatePending}}, }, }, } @@ -50,28 +62,18 @@ func TestPutSuccess(t *testing.T) { } } -func TestPutFailure(t *testing.T) { +func TestPutPipelineValidationFailure(t *testing.T) { type testCase struct { - name string - args []string - source cogito.Source // will be embedded in cogito.PutInput - reader io.Reader // if set, will override fields source above. - writer io.Writer - wantErr string + name string + putInput cogito.PutInput + wantErr string } test := func(t *testing.T, tc testCase) { - assert.Assert(t, tc.wantErr != "") - in := tc.reader - if in == nil { - in = bytes.NewReader(toJSON(t, - cogito.PutInput{ - Source: tc.source, - })) - } log := hclog.NewNullLogger() + in := bytes.NewReader(toJSON(t, tc.putInput)) - err := cogito.Put(log, in, tc.writer, tc.args) + err := cogito.Put(log, in, io.Discard, []string{"dummy-dir"}) assert.Error(t, err, tc.wantErr) } @@ -84,36 +86,115 @@ func TestPutFailure(t *testing.T) { testCases := []testCase{ { - name: "user validation failure: missing keys", - source: cogito.Source{}, - writer: io.Discard, + name: "missing keys", + putInput: cogito.PutInput{ + Source: cogito.Source{}, + }, wantErr: "put: source: missing keys: owner, repo, access_token", }, { - name: "user validation failure: log_level", - source: mergeStructs(baseSource, cogito.Source{LogLevel: "pippo"}), - writer: io.Discard, + name: "invalid log_level", + putInput: cogito.PutInput{ + Source: mergeStructs(baseSource, cogito.Source{LogLevel: "pippo"}), + }, wantErr: "put: source: invalid log_level: pippo", }, { - name: "concourse validation failure: missing input directory", - source: baseSource, - writer: io.Discard, - wantErr: "put: missing input directory from arguments", + name: "invalid params", + putInput: cogito.PutInput{ + Source: baseSource, + Params: cogito.PutParams{State: "burnt-pizza"}, + }, + wantErr: "put: params: invalid build state: burnt-pizza", }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + test(t, tc) + }) + } + +} + +func TestPutProtocolFailure(t *testing.T) { + type testCase struct { + name string + args []string + wantErr string + } + + test := func(t *testing.T, tc testCase) { + log := hclog.NewNullLogger() + in := bytes.NewReader(toJSON(t, cogito.PutInput{ + Source: cogito.Source{ + Owner: "the-owner", + Repo: "the-repo", + AccessToken: "the-token", + }, + Params: cogito.PutParams{State: cogito.StatePending}, + })) + + err := cogito.Put(log, in, io.Discard, tc.args) + + assert.Error(t, err, tc.wantErr) + } + + testCases := []testCase{ { - name: "system write error", - args: []string{"dummy-dir"}, - source: baseSource, - writer: &failingWriter{}, - wantErr: "put: test write error", + name: "missing input directory from arguments", + args: []string{}, + wantErr: "put: arguments: missing input directory", }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + test(t, tc) + }) + } +} + +func TestPutSystemFailure(t *testing.T) { + type testCase struct { + name string + reader io.Reader + writer io.Writer + wantErr string + } + + test := func(t *testing.T, tc testCase) { + assert.Assert(t, tc.wantErr != "") + log := hclog.NewNullLogger() + + err := cogito.Put(log, tc.reader, tc.writer, []string{"dummy-dir"}) + + assert.Error(t, err, tc.wantErr) + } + + baseReader := bytes.NewReader(toJSON(t, + cogito.PutInput{ + Source: cogito.Source{ + Owner: "the-owner", + Repo: "the-repo", + AccessToken: "the-token", + }, + Params: cogito.PutParams{State: cogito.StatePending}, + })) + + testCases := []testCase{ { name: "system read error", reader: iotest.ErrReader(errors.New("test read error")), writer: io.Discard, wantErr: "put: parsing JSON from stdin: test read error", }, + { + name: "system write error", + reader: baseReader, + writer: &failingWriter{}, + wantErr: "put: test write error", + }, } for _, tc := range testCases { @@ -122,3 +203,18 @@ func TestPutFailure(t *testing.T) { }) } } + +// TODO: give a more helpful error message! +// The JSON parser does not mention the outer object ("params") :-( +func TestPutInvalidParamsFailure(t *testing.T) { + in := strings.NewReader(` +{ + "source": {}, + "params": {"pizza": "margherita"} +}`) + wantErr := `put: parsing JSON from stdin: json: unknown field "pizza"` + + err := cogito.Put(hclog.NewNullLogger(), in, io.Discard, []string{}) + + assert.Error(t, err, wantErr) +} diff --git a/resource/resource.go b/resource/resource.go index a071f24b..0e28233f 100644 --- a/resource/resource.go +++ b/resource/resource.go @@ -30,12 +30,6 @@ const ( ownerKey = "owner" repoKey = "repo" stateKey = "state" - - abortState = "abort" - errorState = "error" - failureState = "failure" - pendingState = "pending" - successState = "success" ) var ( From 8354bb92b10a75f8c88d838c2fbfa69f41a5babe Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Tue, 26 Jul 2022 10:57:16 +0200 Subject: [PATCH 5/8] put step: add input dir validation PCI-2587 --- Taskfile.yml | 2 +- cmd/cogito/main_test.go | 2 +- cogito/put.go | 35 +++++ cogito/put_private_test.go | 56 +++++++ cogito/put_test.go | 9 +- .../testdata/a-repo/dot.git/HEAD.template | 0 .../testdata/a-repo/dot.git/config.template | 0 .../refs/heads/{{.branch_name}}.template | 0 .../testdata/empty-dir/.git_keepme | 0 .../testdata/two-dirs/.git_keepme | 0 .../testdata/two-dirs/dir-1/.git_keepme | 0 .../testdata/two-dirs/dir-2/.git_keepme | 0 .../testdata/two-dirs/dir-2/hello | 0 resource/resource.go | 147 +----------------- resource/resource_test.go | 127 --------------- 15 files changed, 101 insertions(+), 277 deletions(-) create mode 100644 cogito/put_private_test.go rename {resource => cogito}/testdata/a-repo/dot.git/HEAD.template (100%) rename {resource => cogito}/testdata/a-repo/dot.git/config.template (100%) rename {resource => cogito}/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template (100%) rename {resource => cogito}/testdata/empty-dir/.git_keepme (100%) rename {resource => cogito}/testdata/two-dirs/.git_keepme (100%) rename {resource => cogito}/testdata/two-dirs/dir-1/.git_keepme (100%) rename {resource => cogito}/testdata/two-dirs/dir-2/.git_keepme (100%) rename {resource => cogito}/testdata/two-dirs/dir-2/hello (100%) diff --git a/Taskfile.yml b/Taskfile.yml index ebfafa73..3d3a3398 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -81,7 +81,7 @@ tasks: - echo - echo '{{.SMOKE_INPUT}}' | ./bin/in dummy-dir - echo - - echo '{{.PUT_INPUT}}' | ./bin/out dummy-dir + - echo '{{.PUT_INPUT}}' | ./bin/out cogito/testdata/a-repo - echo vars: PUT_INPUT: > diff --git a/cmd/cogito/main_test.go b/cmd/cogito/main_test.go index be5443e0..8160de15 100644 --- a/cmd/cogito/main_test.go +++ b/cmd/cogito/main_test.go @@ -60,7 +60,7 @@ func TestRunPutSmokeSuccess(t *testing.T) { var out bytes.Buffer var logOut bytes.Buffer - err := run(in, &out, &logOut, []string{"out", "dummy-dir"}) + err := run(in, &out, &logOut, []string{"out", "../../cogito/testdata/a-repo"}) assert.NilError(t, err, "\nout: %s\nlogOut: %s", out.String(), logOut.String()) } diff --git a/cogito/put.go b/cogito/put.go index 9c2d7b34..4a19d2ab 100644 --- a/cogito/put.go +++ b/cogito/put.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "io" + "os" "github.com/hashicorp/go-hclog" ) @@ -59,6 +60,10 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { } log.Debug("", "state", buildState) + if err := validateInputDir(inputDir, pi.Source.Owner, pi.Source.Repo); err != nil { + return fmt.Errorf("put: validating the input dir: %s", err) + } + // Following the protocol for put, we return the version and metadata. // For Cogito, the metadata contains the Concourse build state. output := Output{ @@ -73,3 +78,33 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { log.Debug("success", "output", output) return nil } + +// validateInputDir checks whether dir, the "put input", conforms to what we expect. +func validateInputDir(dir string, owner string, repo string) error { + inputDirs, err := collectInputDirs(dir) + if err != nil { + return err + } + if len(inputDirs) != 1 { + return fmt.Errorf( + "found %d input dirs: %v. Want exactly 1, corresponding to the GitHub repo %s/%s", + len(inputDirs), inputDirs, owner, repo) + } + + return nil +} + +// collectInputDirs returns a list of all directories below dir (non-recursive). +func collectInputDirs(dir string) ([]string, error) { + entries, err := os.ReadDir(dir) + if err != nil { + return nil, fmt.Errorf("collecting directories in %v: %w", dir, err) + } + var dirs []string + for _, e := range entries { + if e.IsDir() { + dirs = append(dirs, e.Name()) + } + } + return dirs, nil +} diff --git a/cogito/put_private_test.go b/cogito/put_private_test.go new file mode 100644 index 00000000..cf647705 --- /dev/null +++ b/cogito/put_private_test.go @@ -0,0 +1,56 @@ +package cogito + +import ( + "errors" + "os" + "testing" + + "gotest.tools/v3/assert" +) + +func TestValidateInputDirFailure(t *testing.T) { + err := validateInputDir("testdata/two-dirs", "dummy-owner", "dummy-repo") + + assert.Error(t, err, "found 2 input dirs: [dir-1 dir-2]. Want exactly 1, corresponding to the GitHub repo dummy-owner/dummy-repo") +} + +func TestCollectInputDirs(t *testing.T) { + var testCases = []struct { + name string + dir string + wantErr error + wantN int + }{ + { + name: "non existing base directory", + dir: "non-existing", + wantErr: os.ErrNotExist, + wantN: 0, + }, + { + name: "empty directory", + dir: "testdata/empty-dir", + wantErr: nil, + wantN: 0, + }, + { + name: "two directories and one file", + dir: "testdata/two-dirs", + wantErr: nil, + wantN: 2, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + dirs, err := collectInputDirs(tc.dir) + if !errors.Is(err, tc.wantErr) { + t.Errorf("sut(%v): error: have %v; want %v", tc.dir, err, tc.wantErr) + } + gotN := len(dirs) + if gotN != tc.wantN { + t.Errorf("sut(%v): len(dirs): have %v; want %v", tc.dir, gotN, tc.wantN) + } + }) + } +} diff --git a/cogito/put_test.go b/cogito/put_test.go index 2d2bd51a..c420af2b 100644 --- a/cogito/put_test.go +++ b/cogito/put_test.go @@ -25,7 +25,7 @@ func TestPutSuccess(t *testing.T) { var out bytes.Buffer log := hclog.NewNullLogger() - err := cogito.Put(log, in, &out, []string{"dummy-dir"}) + err := cogito.Put(log, in, &out, []string{"testdata/a-repo"}) assert.NilError(t, err) var have cogito.Output @@ -146,6 +146,11 @@ func TestPutProtocolFailure(t *testing.T) { args: []string{}, wantErr: "put: arguments: missing input directory", }, + { + name: "wrong input directory from arguments", + args: []string{"non-existing"}, + wantErr: "put: validating the input dir: collecting directories in non-existing: open non-existing: no such file or directory", + }, } for _, tc := range testCases { @@ -167,7 +172,7 @@ func TestPutSystemFailure(t *testing.T) { assert.Assert(t, tc.wantErr != "") log := hclog.NewNullLogger() - err := cogito.Put(log, tc.reader, tc.writer, []string{"dummy-dir"}) + err := cogito.Put(log, tc.reader, tc.writer, []string{"testdata/a-repo"}) assert.Error(t, err, tc.wantErr) } diff --git a/resource/testdata/a-repo/dot.git/HEAD.template b/cogito/testdata/a-repo/dot.git/HEAD.template similarity index 100% rename from resource/testdata/a-repo/dot.git/HEAD.template rename to cogito/testdata/a-repo/dot.git/HEAD.template diff --git a/resource/testdata/a-repo/dot.git/config.template b/cogito/testdata/a-repo/dot.git/config.template similarity index 100% rename from resource/testdata/a-repo/dot.git/config.template rename to cogito/testdata/a-repo/dot.git/config.template diff --git a/resource/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template b/cogito/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template similarity index 100% rename from resource/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template rename to cogito/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template diff --git a/resource/testdata/empty-dir/.git_keepme b/cogito/testdata/empty-dir/.git_keepme similarity index 100% rename from resource/testdata/empty-dir/.git_keepme rename to cogito/testdata/empty-dir/.git_keepme diff --git a/resource/testdata/two-dirs/.git_keepme b/cogito/testdata/two-dirs/.git_keepme similarity index 100% rename from resource/testdata/two-dirs/.git_keepme rename to cogito/testdata/two-dirs/.git_keepme diff --git a/resource/testdata/two-dirs/dir-1/.git_keepme b/cogito/testdata/two-dirs/dir-1/.git_keepme similarity index 100% rename from resource/testdata/two-dirs/dir-1/.git_keepme rename to cogito/testdata/two-dirs/dir-1/.git_keepme diff --git a/resource/testdata/two-dirs/dir-2/.git_keepme b/cogito/testdata/two-dirs/dir-2/.git_keepme similarity index 100% rename from resource/testdata/two-dirs/dir-2/.git_keepme rename to cogito/testdata/two-dirs/dir-2/.git_keepme diff --git a/resource/testdata/two-dirs/dir-2/hello b/cogito/testdata/two-dirs/dir-2/hello similarity index 100% rename from resource/testdata/two-dirs/dir-2/hello rename to cogito/testdata/two-dirs/dir-2/hello diff --git a/resource/resource.go b/resource/resource.go index 0e28233f..bfd6598a 100644 --- a/resource/resource.go +++ b/resource/resource.go @@ -4,13 +4,10 @@ package resource import ( - "errors" "fmt" - "io/ioutil" "net/url" "os" "path/filepath" - "sort" "strings" "github.com/Pix4D/cogito/github" @@ -25,51 +22,12 @@ const ( contextKey = "context" contextPrefixKey = "context_prefix" - logLevelKey = "log_level" - logUrlKey = "log_url" ownerKey = "owner" repoKey = "repo" stateKey = "state" ) var ( - dummyVersion = oc.Version{"ref": "dummy"} - - outMandatoryParams = map[string]struct{}{ - stateKey: {}, - } - - outOptionalParams = map[string]struct{}{ - contextKey: {}, - } - - outValidStates = map[string]struct{}{ - abortState: {}, - errorState: {}, - failureState: {}, - pendingState: {}, - successState: {}, - } - - mandatorySourceKeys = map[string]struct{}{ - ownerKey: {}, - repoKey: {}, - accessTokenKey: {}, - } - - optionalSourceKeys = map[string]struct{}{ - logLevelKey: {}, - logUrlKey: {}, - contextPrefixKey: {}, - // - gchatWebhookKey: {}, - } - - secretKeys = map[string]struct{}{ - accessTokenKey: {}, - gchatWebhookKey: {}, - } - // States that will trigger a chat notification by default. statesToNotifyChat = []string{abortState, errorState, failureState} ) @@ -106,35 +64,8 @@ func (r *Resource) Out( env oc.Environment, log *oc.Logger, ) (oc.Version, oc.Metadata, error) { - log.Debugf("out: started") - defer log.Debugf("out: finished") - log.Infof(BuildInfo()) - log.Debugf("out: source:\n%s", stringify(redact(source, secretKeys))) - log.Debugf("out: params:\n%s", stringify(params)) - log.Debugf("out: env:\n%s", stringify(env.GetAll())) - - if err := validateSource(source); err != nil { - return nil, nil, err - } - - if err := validateOutParams(params); err != nil { - return nil, nil, err - } - - owner, _ := source[ownerKey].(string) - repo, _ := source[repoKey].(string) - - inputDirs, err := collectInputDirs(inputDir) - if err != nil { - return nil, nil, err - } - if len(inputDirs) != 1 { - err := fmt.Errorf("found %d input dirs: %v. "+ - "Want exactly 1, corresponding to the GitHub repo %s/%s", - len(inputDirs), inputDirs, owner, repo) - return nil, nil, err - } + // STUFF DELETED repoDir := filepath.Join(inputDir, inputDirs[0]) if err := checkRepoDir(repoDir, owner, repo); err != nil { @@ -180,82 +111,6 @@ func (r *Resource) Out( return dummyVersion, metadata, nil } -func validateSource(source oc.Source) error { - // Any missing source key? - missing := make([]string, 0, len(mandatorySourceKeys)) - for key := range mandatorySourceKeys { - if _, ok := source[key].(string); !ok { - missing = append(missing, key) - } - } - - // Any unknown source key? - unknown := make([]string, 0, len(source)) - for key := range source { - _, ok1 := mandatorySourceKeys[key] - _, ok2 := optionalSourceKeys[key] - if !ok1 && !ok2 { - unknown = append(unknown, key) - } - } - - errMsg := make([]string, 0, 2) - if len(missing) > 0 { - sort.Strings(missing) - errMsg = append(errMsg, fmt.Sprintf("missing source keys: %s", missing)) - } - if len(unknown) > 0 { - sort.Strings(unknown) - errMsg = append(errMsg, fmt.Sprintf("unknown source keys: %s", unknown)) - } - if len(errMsg) > 0 { - return errors.New(strings.Join(errMsg, "; ")) - } - - return nil -} - -func validateOutParams(params oc.Params) error { - // Any missing parameter? - for wantParam := range outMandatoryParams { - if _, ok := params[wantParam].(string); !ok { - return fmt.Errorf("missing put parameter '%s'", wantParam) - } - } - - // Any invalid parameter? - state, _ := params[stateKey].(string) - if _, ok := outValidStates[state]; !ok { - return fmt.Errorf("invalid put parameter 'state: %s'", state) - } - - // Any unknown parameter? - for param := range params { - _, ok1 := outMandatoryParams[param] - _, ok2 := outOptionalParams[param] - if !ok1 && !ok2 { - return fmt.Errorf("unknown put parameter '%s'", param) - } - } - - return nil -} - -// Return a list of all directories below dir (non-recursive). -func collectInputDirs(dir string) ([]string, error) { - entries, err := ioutil.ReadDir(dir) - if err != nil { - return nil, fmt.Errorf("collecting directories in %v: %w", dir, err) - } - dirs := []string{} - for _, e := range entries { - if e.IsDir() { - dirs = append(dirs, e.Name()) - } - } - return dirs, nil -} - // checkRepoDir validates whether DIR, assumed to be received as input of a put step, // contains a git repository usable with the Cogito source configuration: // - DIR is indeed a git repository. diff --git a/resource/resource_test.go b/resource/resource_test.go index 094e1eb9..5f6242c7 100644 --- a/resource/resource_test.go +++ b/resource/resource_test.go @@ -2,7 +2,6 @@ package resource import ( "encoding/json" - "errors" "fmt" "io" "net/http" @@ -56,9 +55,6 @@ func TestOutMockSuccess(t *testing.T) { wantMeta oc.Metadata wantBody map[string]string }{ - { - name: "valid mandatory source and params", - }, { name: "source: optional: context_prefix", source: help.MergeMap(defSource, oc.Source{ @@ -154,88 +150,6 @@ func TestOutMockSuccess(t *testing.T) { } } -func TestOutMockFailure(t *testing.T) { - cfg := help.FakeTestCfg - - defSource := oc.Source{ - accessTokenKey: cfg.Token, - ownerKey: cfg.Owner, - repoKey: cfg.Repo, - } - defParams := oc.Params{ - stateKey: errorState, - } - - testDir := "a-repo" - - testCases := []struct { - name string - source oc.Source - params oc.Params - wantErr string - }{ - { - name: "missing mandatory source keys", - source: oc.Source{}, - params: defParams, - wantErr: "missing source keys: [access_token owner repo]", - }, - { - name: "missing mandatory parameters", - source: defSource, - params: oc.Params{}, - wantErr: "missing put parameter 'state'", - }, - { - name: "invalid state parameter", - source: defSource, - params: oc.Params{ - stateKey: "hello", - }, - wantErr: "invalid put parameter 'state: hello'", - }, - { - name: "unknown parameter", - source: defSource, - params: oc.Params{ - stateKey: pendingState, - "pizza": "margherita", - }, - wantErr: "unknown put parameter 'pizza'", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - inDir := setup(t, testDir, sshRemote(cfg.Owner, cfg.Repo), cfg.SHA, cfg.SHA) - - ts := httptest.NewServer( - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusCreated) - fmt.Fprintln(w, "Anything goes...") - }), - ) - - defer func() { - ts.Close() - }() - - res := NewWith(ts.URL) - - _, _, err := res.Out( - inDir, tc.source, tc.params, defEnv, silentLog) - - if err == nil { - t.Fatalf("\nhave: \nwant: %s", tc.wantErr) - } - - if diff := cmp.Diff(tc.wantErr, err.Error()); diff != "" { - t.Fatalf("error msg mismatch: (-want +have):\n%s", diff) - } - }) - } -} - func TestOutSuccessIntegration(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") @@ -348,47 +262,6 @@ Cogito SOURCE configuration: } } -func TestCollectInputDirs(t *testing.T) { - var testCases = []struct { - name string - dir string - wantErr error - wantN int - }{ - { - name: "non existing base directory", - dir: "non-existing", - wantErr: os.ErrNotExist, - wantN: 0, - }, - { - name: "empty directory", - dir: "testdata/empty-dir", - wantErr: nil, - wantN: 0, - }, - { - name: "two directories and one file", - dir: "testdata/two-dirs", - wantErr: nil, - wantN: 2, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - dirs, err := collectInputDirs(tc.dir) - if !errors.Is(err, tc.wantErr) { - t.Errorf("sut(%v): error: have %v; want %v", tc.dir, err, tc.wantErr) - } - gotN := len(dirs) - if gotN != tc.wantN { - t.Errorf("sut(%v): len(dirs): have %v; want %v", tc.dir, gotN, tc.wantN) - } - }) - } -} - func TestCheckRepoDirSuccess(t *testing.T) { const wantOwner = "smiling" const wantRepo = "butterfly" From f272b1a0dd846494a202dd34e8ac8a91aed03492 Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Tue, 26 Jul 2022 16:49:18 +0200 Subject: [PATCH 6/8] put step: input dir: validate git repo PCI-2587 --- Taskfile.yml | 10 +- cmd/cogito/main_test.go | 5 +- cmd/copydir/main.go | 21 +- cogito/put.go | 124 +++++++- cogito/put_private_test.go | 276 ++++++++++++++++- cogito/put_test.go | 11 +- .../testdata/not-a-repo/a-dir/.git_keepme | 0 .../testdata/not-a-repo/a-file | 0 .../repo-bad-git-config/dot.git/config | 0 github/commitstatus_test.go | 10 +- go.mod | 8 +- go.sum | 10 + resource/resource.go | 126 +------- resource/resource_test.go | 284 ------------------ {help => testhelp}/testhelper.go | 65 +++- 15 files changed, 509 insertions(+), 441 deletions(-) rename resource/testdata/not-a-repo/a-file => cogito/testdata/not-a-repo/a-dir/.git_keepme (100%) rename resource/testdata/repo-bad-git-config/dot.git/config => cogito/testdata/not-a-repo/a-file (100%) create mode 100644 cogito/testdata/repo-bad-git-config/dot.git/config rename {help => testhelp}/testhelper.go (72%) diff --git a/Taskfile.yml b/Taskfile.yml index 3d3a3398..2ca7d24c 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -76,12 +76,20 @@ tasks: test:smoke: desc: Simple smoke test of the local executables. cmds: + - task: build + - task: build:copydir - task: test:buildinfo - echo '{{.SMOKE_INPUT}}' | ./bin/check - echo - echo '{{.SMOKE_INPUT}}' | ./bin/in dummy-dir - echo - - echo '{{.PUT_INPUT}}' | ./bin/out cogito/testdata/a-repo + - rm -rf /tmp/cogito-test + - mkdir -p /tmp/cogito-test + - > + ./bin/copydir cogito/testdata/a-repo /tmp/cogito-test --dot + --template repo_url=https://github.com/foo/bar head=dummyHead + branch_name=dummyBranch commit_sha=dummySha + - echo '{{.PUT_INPUT}}' | ./bin/out /tmp/cogito-test - echo vars: PUT_INPUT: > diff --git a/cmd/cogito/main_test.go b/cmd/cogito/main_test.go index 8160de15..f222fa10 100644 --- a/cmd/cogito/main_test.go +++ b/cmd/cogito/main_test.go @@ -6,6 +6,7 @@ import ( "strings" "testing" + "github.com/Pix4D/cogito/testhelp" "gotest.tools/v3/assert" ) @@ -59,8 +60,10 @@ func TestRunPutSmokeSuccess(t *testing.T) { }`) var out bytes.Buffer var logOut bytes.Buffer + inputDir := testhelp.MakeGitRepoFromTestdata(t, "../../cogito/testdata/a-repo", + testhelp.HttpsRemote("the-owner", "the-repo"), "dummySHA", "dummyHead") - err := run(in, &out, &logOut, []string{"out", "../../cogito/testdata/a-repo"}) + err := run(in, &out, &logOut, []string{"out", inputDir}) assert.NilError(t, err, "\nout: %s\nlogOut: %s", out.String(), logOut.String()) } diff --git a/cmd/copydir/main.go b/cmd/copydir/main.go index 9e7f81d4..ebfeaa69 100644 --- a/cmd/copydir/main.go +++ b/cmd/copydir/main.go @@ -1,4 +1,4 @@ -// Useful when developing help.CopyDir. +// Useful when developing testhelp.CopyDir. package main import ( @@ -6,9 +6,9 @@ import ( "os" "strings" - arg "github.com/alexflint/go-arg" + "github.com/alexflint/go-arg" - "github.com/Pix4D/cogito/help" + "github.com/Pix4D/cogito/testhelp" ) func main() { @@ -24,26 +24,23 @@ func main() { fmt.Println("error:", err) os.Exit(1) } - src := os.Args[1] - dst := os.Args[2] - fmt.Println("src:", src, "dst:", dst) - var renamer help.Renamer + var renamer testhelp.Renamer if args.Dot { - renamer = help.DotRenamer + renamer = testhelp.DotRenamer } else { - renamer = help.IdentityRenamer + renamer = testhelp.IdentityRenamer } - if err := help.CopyDir(dst, src, renamer, templateData); err != nil { + if err := testhelp.CopyDir(args.Dst, args.Src, renamer, templateData); err != nil { fmt.Println("error:", err) os.Exit(1) } } // Take a list of strings of the form "key=value" and convert them to map entries. -func makeTemplateData(keyvals []string) (help.TemplateData, error) { - data := help.TemplateData{} +func makeTemplateData(keyvals []string) (testhelp.TemplateData, error) { + data := testhelp.TemplateData{} for _, keyval := range keyvals { pos := strings.Index(keyval, "=") if pos == -1 { diff --git a/cogito/put.go b/cogito/put.go index 4a19d2ab..79a7312f 100644 --- a/cogito/put.go +++ b/cogito/put.go @@ -4,9 +4,13 @@ import ( "encoding/json" "fmt" "io" + "net/url" "os" + "path/filepath" + "strings" "github.com/hashicorp/go-hclog" + "github.com/sasbury/mini" ) // Put implements the "put" step (the "out" executable). @@ -79,9 +83,10 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { return nil } -// validateInputDir checks whether dir, the "put input", conforms to what we expect. -func validateInputDir(dir string, owner string, repo string) error { - inputDirs, err := collectInputDirs(dir) +// validateInputDir checks whether dir, containing the "put inputs", conforms +// to what we expect. +func validateInputDir(inputDir string, owner string, repo string) error { + inputDirs, err := collectInputDirs(inputDir) if err != nil { return err } @@ -91,6 +96,11 @@ func validateInputDir(dir string, owner string, repo string) error { len(inputDirs), inputDirs, owner, repo) } + repoDir := filepath.Join(inputDir, inputDirs[0]) + if err := checkGitRepoDir(repoDir, owner, repo); err != nil { + return err + } + return nil } @@ -108,3 +118,111 @@ func collectInputDirs(dir string) ([]string, error) { } return dirs, nil } + +// checkGitRepoDir validates whether DIR, assumed to be received as input of a put step, +// contains a git repository usable with the Cogito source configuration: +// - DIR is indeed a git repository. +// - The repo configuration contains a "remote origin" section. +// - The remote origin url can be parsed following the GitHub conventions. +// - The result of the parse matches OWNER and REPO. +func checkGitRepoDir(dir, owner, repo string) error { + cfg, err := mini.LoadConfiguration(filepath.Join(dir, ".git/config")) + if err != nil { + return fmt.Errorf("parsing .git/config: %w", err) + } + + // .git/config contains a section like: + // + // [remote "origin"] + // url = git@github.com:Pix4D/cogito.git + // fetch = +refs/heads/*:refs/remotes/origin/* + // + const section = `remote "origin"` + const key = "url" + gitUrl := cfg.StringFromSection(section, key, "") + if gitUrl == "" { + return fmt.Errorf(".git/config: key [%s]/%s: not found", section, key) + } + gu, err := parseGitPseudoURL(gitUrl) + if err != nil { + return fmt.Errorf(".git/config: remote: %w", err) + } + left := []string{"github.com", owner, repo} + right := []string{gu.URL.Host, gu.Owner, gu.Repo} + for i, l := range left { + r := right[i] + if !strings.EqualFold(l, r) { + return fmt.Errorf(`the received git repository is incompatible with the Cogito configuration. + +Git repository configuration (received as 'inputs:' in this PUT step): + url: %s + owner: %s + repo: %s + +Cogito SOURCE configuration: + owner: %s + repo: %s`, + gitUrl, gu.Owner, gu.Repo, + owner, repo) + } + } + return nil +} + +type gitURL struct { + URL *url.URL + Owner string + Repo string +} + +// parseGitPseudoURL attempts to parse rawURL as a git remote URL compatible with the +// Github naming conventions. +// +// It supports the following types of git pseudo URLs: +// - ssh: git@github.com:Pix4D/cogito.git; will be rewritten to the valid URL +// ssh://git@github.com/Pix4D/cogito.git +// - https: https://github.com/Pix4D/cogito.git +// - http: http://github.com/Pix4D/cogito.git +func parseGitPseudoURL(rawURL string) (gitURL, error) { + workURL := rawURL + // If ssh pseudo URL, we need to massage the rawURL ourselves :-( + if strings.HasPrefix(workURL, "git@") { + if strings.Count(workURL, ":") != 1 { + return gitURL{}, fmt.Errorf("invalid git SSH URL %s: want exactly one ':'", rawURL) + } + // Make the URL a real URL, ready to be parsed. For example: + // git@github.com:Pix4D/cogito.git -> ssh://git@github.com/Pix4D/cogito.git + workURL = "ssh://" + strings.Replace(workURL, ":", "/", 1) + } + + anyUrl, err := url.Parse(workURL) + if err != nil { + return gitURL{}, err + } + + scheme := anyUrl.Scheme + if scheme == "" { + return gitURL{}, fmt.Errorf("invalid git URL %s: missing scheme", rawURL) + } + if scheme != "ssh" && scheme != "http" && scheme != "https" { + return gitURL{}, fmt.Errorf("invalid git URL %s: invalid scheme: %s", rawURL, scheme) + } + + // Further parse the path component of the URL to see if it complies with the GitHub + // naming conventions. + // Example of compliant path: github.com/Pix4D/cogito.git + tokens := strings.Split(anyUrl.Path, "/") + if have, want := len(tokens), 3; have != want { + return gitURL{}, + fmt.Errorf("invalid git URL: path: want: %d components; have: %d %s", + want, have, tokens) + } + + // All OK. Fill our gitURL struct + gu := gitURL{ + URL: anyUrl, + Owner: tokens[1], + Repo: strings.TrimSuffix(tokens[2], ".git"), + } + return gu, nil +} diff --git a/cogito/put_private_test.go b/cogito/put_private_test.go index cf647705..95c0675e 100644 --- a/cogito/put_private_test.go +++ b/cogito/put_private_test.go @@ -2,16 +2,53 @@ package cogito import ( "errors" + "net/url" "os" + "path/filepath" "testing" + "github.com/Pix4D/cogito/testhelp" + "github.com/gertd/wild" + "github.com/google/go-cmp/cmp" "gotest.tools/v3/assert" ) func TestValidateInputDirFailure(t *testing.T) { - err := validateInputDir("testdata/two-dirs", "dummy-owner", "dummy-repo") + type testCase struct { + name string + inputDir string + wantErr string + } + + test := func(t *testing.T, tc testCase) { + tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.inputDir, + "https://github.com/foo", "dummySHA", "dummyHead") + + err := validateInputDir(filepath.Join(tmpDir, filepath.Base(tc.inputDir)), + "dummy-owner", "dummy-repo") + + assert.ErrorContains(t, err, tc.wantErr) + } + + testCases := []testCase{ + { + name: "two input dirs", + inputDir: "testdata/two-dirs", + wantErr: "found 2 input dirs: [dir-1 dir-2]. Want exactly 1, corresponding to the GitHub repo dummy-owner/dummy-repo", + }, + { + name: "one input dir but not a repo", + inputDir: "testdata/not-a-repo", + wantErr: "parsing .git/config: open ", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + test(t, tc) + }) + } - assert.Error(t, err, "found 2 input dirs: [dir-1 dir-2]. Want exactly 1, corresponding to the GitHub repo dummy-owner/dummy-repo") } func TestCollectInputDirs(t *testing.T) { @@ -54,3 +91,238 @@ func TestCollectInputDirs(t *testing.T) { }) } } + +func TestCheckGitRepoDirSuccess(t *testing.T) { + const wantOwner = "smiling" + const wantRepo = "butterfly" + + testCases := []struct { + name string + dir string // repoURL to put in file /.git/config + repoURL string + }{ + { + name: "repo with good SSH remote", + dir: "a-repo", + repoURL: testhelp.SshRemote(wantOwner, wantRepo), + }, + { + name: "repo with good HTTPS remote", + dir: "a-repo", + repoURL: testhelp.HttpsRemote(wantOwner, wantRepo), + }, + { + name: "repo with good HTTP remote", + dir: "a-repo", + repoURL: testhelp.HttpRemote(wantOwner, wantRepo), + }, + { + name: "PR resource but with basic auth in URL (see PR #46)", + dir: "a-repo", + repoURL: "https://x-oauth-basic:ghp_XXX@github.com/smiling/butterfly.git", + }, + } + + for _, tc := range testCases { + inputDir := testhelp.MakeGitRepoFromTestdata(t, filepath.Join("testdata", tc.dir), + tc.repoURL, "dummySHA", "dummyHead") + + t.Run(tc.name, func(t *testing.T) { + err := checkGitRepoDir(filepath.Join(inputDir, tc.dir), wantOwner, wantRepo) + + assert.NilError(t, err) + }) + } +} + +func TestCheckGitRepoDirFailure(t *testing.T) { + const wantOwner = "smiling" + const wantRepo = "butterfly" + + testCases := []struct { + name string + dir string + repoURL string // repoURL to put in file /.git/config + wantErrWild string // wildcard matching + }{ + { + name: "dir is not a repo", + dir: "not-a-repo", + repoURL: "dummyurl", + wantErrWild: `parsing .git/config: open */not-a-repo/.git/config: no such file or directory`, + }, + { + name: "bad file .git/config", + dir: "repo-bad-git-config", + repoURL: "dummyurl", + wantErrWild: `.git/config: key [remote "origin"]/url: not found`, + }, + { + name: "repo with unrelated HTTPS remote", + dir: "a-repo", + repoURL: testhelp.HttpsRemote("owner-a", "repo-a"), + wantErrWild: `the received git repository is incompatible with the Cogito configuration. + +Git repository configuration (received as 'inputs:' in this PUT step): + url: https://github.com/owner-a/repo-a.git + owner: owner-a + repo: repo-a + +Cogito SOURCE configuration: + owner: smiling + repo: butterfly`, + }, + { + name: "repo with unrelated SSH remote or wrong source config", + dir: "a-repo", + repoURL: testhelp.SshRemote("owner-a", "repo-a"), + wantErrWild: `the received git repository is incompatible with the Cogito configuration. + +Git repository configuration (received as 'inputs:' in this PUT step): + url: git@github.com:owner-a/repo-a.git + owner: owner-a + repo: repo-a + +Cogito SOURCE configuration: + owner: smiling + repo: butterfly`, + }, + { + name: "invalid git pseudo URL in .git/config", + dir: "a-repo", + repoURL: "foo://bar", + wantErrWild: `.git/config: remote: invalid git URL foo://bar: invalid scheme: foo`, + }, + } + + for _, tc := range testCases { + inDir := testhelp.MakeGitRepoFromTestdata(t, filepath.Join("testdata", tc.dir), + tc.repoURL, "dummySHA", "dummyHead") + + t.Run(tc.name, func(t *testing.T) { + err := checkGitRepoDir(filepath.Join(inDir, tc.dir), wantOwner, wantRepo) + + if err == nil { + t.Fatalf("\nhave: \nwant: %s", tc.wantErrWild) + } + + have := err.Error() + if !wild.Match(tc.wantErrWild, have, false) { + diff := cmp.Diff(tc.wantErrWild, have) + t.Fatalf("error msg wildcard mismatch: (-want +have):\n%s", diff) + } + }) + } +} + +func TestParseGitPseudoURLSuccess(t *testing.T) { + testCases := []struct { + name string + inURL string + wantGU gitURL + }{ + { + name: "valid SSH URL", + inURL: "git@github.com:Pix4D/cogito.git", + wantGU: gitURL{ + URL: &url.URL{ + Scheme: "ssh", + User: url.User("git"), + Host: "github.com", + Path: "/Pix4D/cogito.git", + }, + Owner: "Pix4D", + Repo: "cogito", + }, + }, + { + name: "valid HTTPS URL", + inURL: "https://github.com/Pix4D/cogito.git", + wantGU: gitURL{ + URL: &url.URL{ + Scheme: "https", + Host: "github.com", + Path: "/Pix4D/cogito.git", + }, + Owner: "Pix4D", + Repo: "cogito", + }, + }, + { + name: "valid HTTP URL", + inURL: "http://github.com/Pix4D/cogito.git", + wantGU: gitURL{ + URL: &url.URL{ + Scheme: "http", + Host: "github.com", + Path: "/Pix4D/cogito.git", + }, + Owner: "Pix4D", + Repo: "cogito", + }, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + gitUrl, err := parseGitPseudoURL(tc.inURL) + + if err != nil { + t.Fatalf("\nhave: %s\nwant: ", err) + } + if diff := cmp.Diff(tc.wantGU, gitUrl, cmp.Comparer( + func(x, y *url.Userinfo) bool { + return x.String() == y.String() + })); diff != "" { + t.Errorf("gitURL: (-want +have):\n%s", diff) + } + }) + } +} + +func TestParseGitPseudoURLFailure(t *testing.T) { + testCases := []struct { + name string + inURL string + wantErr string + }{ + { + name: "totally invalid URL", + inURL: "hello", + wantErr: "invalid git URL hello: missing scheme", + }, + { + name: "invalid SSH URL", + inURL: "git@github.com/Pix4D/cogito.git", + wantErr: "invalid git SSH URL git@github.com/Pix4D/cogito.git: want exactly one ':'", + }, + { + name: "invalid HTTPS URL", + inURL: "https://github.com:Pix4D/cogito.git", + wantErr: `parse "https://github.com:Pix4D/cogito.git": invalid port ":Pix4D" after host`, + }, + { + name: "invalid HTTP URL", + inURL: "http://github.com:Pix4D/cogito.git", + wantErr: `parse "http://github.com:Pix4D/cogito.git": invalid port ":Pix4D" after host`, + }, + { + name: "too few path components", + inURL: "http://github.com/cogito.git", + wantErr: "invalid git URL: path: want: 3 components; have: 2 [ cogito.git]", + }, + { + name: "too many path components", + inURL: "http://github.com/1/2/cogito.git", + wantErr: "invalid git URL: path: want: 3 components; have: 4 [ 1 2 cogito.git]", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + _, err := parseGitPseudoURL(tc.inURL) + + assert.Error(t, err, tc.wantErr) + }) + } +} diff --git a/cogito/put_test.go b/cogito/put_test.go index c420af2b..2572c45c 100644 --- a/cogito/put_test.go +++ b/cogito/put_test.go @@ -9,6 +9,7 @@ import ( "testing/iotest" "github.com/Pix4D/cogito/cogito" + "github.com/Pix4D/cogito/testhelp" "github.com/hashicorp/go-hclog" "gotest.tools/v3/assert" ) @@ -24,8 +25,10 @@ func TestPutSuccess(t *testing.T) { in := bytes.NewReader(toJSON(t, tc.in)) var out bytes.Buffer log := hclog.NewNullLogger() + inputDir := testhelp.MakeGitRepoFromTestdata(t, "testdata/a-repo", + testhelp.HttpsRemote("the-owner", "the-repo"), "dummySHA", "dummyHead") - err := cogito.Put(log, in, &out, []string{"testdata/a-repo"}) + err := cogito.Put(log, in, &out, []string{inputDir}) assert.NilError(t, err) var have cogito.Output @@ -171,8 +174,10 @@ func TestPutSystemFailure(t *testing.T) { test := func(t *testing.T, tc testCase) { assert.Assert(t, tc.wantErr != "") log := hclog.NewNullLogger() + inputDir := testhelp.MakeGitRepoFromTestdata(t, "testdata/a-repo", + testhelp.HttpsRemote("the-owner", "the-repo"), "dummySHA", "dummyHead") - err := cogito.Put(log, tc.reader, tc.writer, []string{"testdata/a-repo"}) + err := cogito.Put(log, tc.reader, tc.writer, []string{inputDir}) assert.Error(t, err, tc.wantErr) } @@ -209,8 +214,6 @@ func TestPutSystemFailure(t *testing.T) { } } -// TODO: give a more helpful error message! -// The JSON parser does not mention the outer object ("params") :-( func TestPutInvalidParamsFailure(t *testing.T) { in := strings.NewReader(` { diff --git a/resource/testdata/not-a-repo/a-file b/cogito/testdata/not-a-repo/a-dir/.git_keepme similarity index 100% rename from resource/testdata/not-a-repo/a-file rename to cogito/testdata/not-a-repo/a-dir/.git_keepme diff --git a/resource/testdata/repo-bad-git-config/dot.git/config b/cogito/testdata/not-a-repo/a-file similarity index 100% rename from resource/testdata/repo-bad-git-config/dot.git/config rename to cogito/testdata/not-a-repo/a-file diff --git a/cogito/testdata/repo-bad-git-config/dot.git/config b/cogito/testdata/repo-bad-git-config/dot.git/config new file mode 100644 index 00000000..e69de29b diff --git a/github/commitstatus_test.go b/github/commitstatus_test.go index b9eb640e..c95e0a3f 100644 --- a/github/commitstatus_test.go +++ b/github/commitstatus_test.go @@ -9,12 +9,12 @@ import ( "time" "github.com/Pix4D/cogito/github" - "github.com/Pix4D/cogito/help" + "github.com/Pix4D/cogito/testhelp" "github.com/google/go-cmp/cmp" ) func TestGitHubStatusSuccessMockAPI(t *testing.T) { - cfg := help.FakeTestCfg + cfg := testhelp.FakeTestCfg context := "cogito/test" targetURL := "https://cogito.invalid/builds/job/42" desc := time.Now().Format("15:04:05") @@ -51,7 +51,7 @@ func TestGitHubStatusSuccessMockAPI(t *testing.T) { } func TestGitHubStatusFailureMockAPI(t *testing.T) { - cfg := help.FakeTestCfg + cfg := testhelp.FakeTestCfg context := "cogito/test" targetURL := "https://cogito.invalid/builds/job/42" desc := time.Now().Format("15:04:05") @@ -133,7 +133,7 @@ func TestGitHubStatusSuccessIntegration(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - cfg := help.SkipTestIfNoEnvVars(t) + cfg := testhelp.SkipTestIfNoEnvVars(t) context := "cogito/test" targetURL := "https://cogito.invalid/builds/job/42" desc := time.Now().Format("15:04:05") @@ -151,7 +151,7 @@ func TestGitHubStatusFailureIntegration(t *testing.T) { if testing.Short() { t.Skip("skipping integration test") } - cfg := help.SkipTestIfNoEnvVars(t) + cfg := testhelp.SkipTestIfNoEnvVars(t) state := "success" testCases := []struct { diff --git a/go.mod b/go.mod index 8f5d0263..44fdebd1 100644 --- a/go.mod +++ b/go.mod @@ -3,20 +3,20 @@ module github.com/Pix4D/cogito go 1.18 require ( - github.com/alexflint/go-arg v1.3.0 + github.com/alexflint/go-arg v1.4.3 github.com/cloudboss/ofcourse v0.2.2 github.com/gertd/wild v0.0.2-0.20200810044159-389fd0bed676 github.com/google/go-cmp v0.5.8 - github.com/hashicorp/go-hclog v1.2.1 + github.com/hashicorp/go-hclog v1.2.2 github.com/imdario/mergo v0.3.13 github.com/sasbury/mini v0.0.0-20181226232755-dc74af49394b gotest.tools/v3 v3.3.0 ) require ( - github.com/alexflint/go-scalar v1.0.0 // indirect + github.com/alexflint/go-scalar v1.1.0 // indirect github.com/fatih/color v1.13.0 // indirect github.com/mattn/go-colorable v0.1.12 // indirect github.com/mattn/go-isatty v0.0.14 // indirect - golang.org/x/sys v0.0.0-20220712014510-0a85c31ab51e // indirect + golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f // indirect ) diff --git a/go.sum b/go.sum index 3d7e2e37..e5434573 100644 --- a/go.sum +++ b/go.sum @@ -5,8 +5,12 @@ github.com/aclements/go-moremath v0.0.0-20161014184102-0ff62e0875ff/go.mod h1:id github.com/ajstarks/svgo v0.0.0-20200725142600-7a3c8b57fecb/go.mod h1:K08gAheRH3/J6wwsYMMT4xOr94bZjxIelGM0+d/wbFw= github.com/alexflint/go-arg v1.3.0 h1:UfldqSdFWeLtoOuVRosqofU4nmhI1pYEbT4ZFS34Bdo= github.com/alexflint/go-arg v1.3.0/go.mod h1:9iRbDxne7LcR/GSvEr7ma++GLpdIU1zrghf2y2768kM= +github.com/alexflint/go-arg v1.4.3 h1:9rwwEBpMXfKQKceuZfYcwuc/7YY7tWJbFsgG5cAU/uo= +github.com/alexflint/go-arg v1.4.3/go.mod h1:3PZ/wp/8HuqRZMUUgu7I+e1qcpUbvmS258mRXkFH4IA= github.com/alexflint/go-scalar v1.0.0 h1:NGupf1XV/Xb04wXskDFzS0KWOLH632W/EO4fAFi+A70= github.com/alexflint/go-scalar v1.0.0/go.mod h1:GpHzbCOZXEKMEcygYQ5n/aa4Aq84zbxjy3MxYW0gjYw= +github.com/alexflint/go-scalar v1.1.0 h1:aaAouLLzI9TChcPXotr6gUhq+Scr8rl0P9P4PnltbhM= +github.com/alexflint/go-scalar v1.1.0/go.mod h1:LoFvNMqS1CPrMVltza4LvnGKhaSpc3oyLEBUZVhhS2o= github.com/cloudboss/ofcourse v0.2.2 h1:FrSvCwfeYnWhSwAmI6ub9yDDXbDyAB+HvFy98lFr5Us= github.com/cloudboss/ofcourse v0.2.2/go.mod h1:xSUlHhdjOZt8jOJR610vADmAUXhG8SiFyUBhe9Su8Rs= github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= @@ -33,6 +37,8 @@ github.com/google/shlex v0.0.0-20191202100458-e7afc7fbc510/go.mod h1:pupxD2MaaD3 github.com/googleapis/gax-go v0.0.0-20161107002406-da06d194a00e/go.mod h1:SFVmujtThgffbyetf+mdk2eWhX2bMyUtNHzFKcPA9HY= github.com/hashicorp/go-hclog v1.2.1 h1:YQsLlGDJgwhXFpucSPyVbCBviQtjlHv3jLTlp8YmtEw= github.com/hashicorp/go-hclog v1.2.1/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/hashicorp/go-hclog v1.2.2 h1:ihRI7YFwcZdiSD7SIenIhHfQH3OuDvWerAUBZbeQS3M= +github.com/hashicorp/go-hclog v1.2.2/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= github.com/imdario/mergo v0.3.13 h1:lFzP57bqS/wsqKssCGmtLAb8A0wKjLGrve2q3PPVcBk= github.com/imdario/mergo v0.3.13/go.mod h1:4lJ1jqUDcsbIECGy0RUJAXNIhg+6ocWgb1ALK2O4oXg= github.com/inconshreveable/mousetrap v1.0.0/go.mod h1:PxqpIevigyE2G7u3NXJIT2ANytuPF1OarO4DADm73n8= @@ -61,6 +67,7 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= 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.7.2 h1:4jaiDzPyXQvSd7D0EjG45355tLlV3VOECpq10pLC+8s= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= github.com/yuin/goldmark v1.1.32/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= @@ -98,6 +105,8 @@ golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220712014510-0a85c31ab51e h1:NHvCuwuS43lGnYhten69ZWqi2QOj/CiDNcKbVqwVoew= golang.org/x/sys v0.0.0-20220712014510-0a85c31ab51e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f h1:v4INt8xihDGvnrfjMDVXGxw9wrfxYyCjk0KbXjhR55s= +golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -113,6 +122,7 @@ google.golang.org/api v0.0.0-20170206182103-3d017632ea10/go.mod h1:4mhQ8q/RsB7i+ google.golang.org/appengine v1.4.0/go.mod h1:xpcJRLb0r/rnEns0DIKYYv+WjYCduHsrkT7/EB5XEv4= google.golang.org/grpc v0.0.0-20170208002647-2a6bf6142e96/go.mod h1:yo6s7OP7yaDglbqo1J04qKzAhqBH6lvTonzMVmEdcZw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= +gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.0/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= diff --git a/resource/resource.go b/resource/resource.go index bfd6598a..d8014937 100644 --- a/resource/resource.go +++ b/resource/resource.go @@ -5,14 +5,11 @@ package resource import ( "fmt" - "net/url" "os" "path/filepath" "strings" "github.com/Pix4D/cogito/github" - "github.com/sasbury/mini" - oc "github.com/cloudboss/ofcourse/ofcourse" ) @@ -67,12 +64,7 @@ func (r *Resource) Out( // STUFF DELETED - repoDir := filepath.Join(inputDir, inputDirs[0]) - if err := checkRepoDir(repoDir, owner, repo); err != nil { - return nil, nil, err - } - - gitRef, err := GitGetCommit(repoDir) + gitRef, err := getGitCommit(repoDir) if err != nil { return nil, nil, err } @@ -111,120 +103,8 @@ func (r *Resource) Out( return dummyVersion, metadata, nil } -// checkRepoDir validates whether DIR, assumed to be received as input of a put step, -// contains a git repository usable with the Cogito source configuration: -// - DIR is indeed a git repository. -// - The repo configuration contains a "remote origin" section. -// - The remote origin url can be parsed following the Github conventions. -// - The result of the parse matches OWNER and REPO. -func checkRepoDir(dir, owner, repo string) error { - dir, err := filepath.Abs(dir) - if err != nil { - return fmt.Errorf("parsing .git/config: abspath: %w", err) - } - cfg, err := mini.LoadConfiguration(filepath.Join(dir, ".git/config")) - if err != nil { - return fmt.Errorf("parsing .git/config: %w", err) - } - - // .git/config contains a section like: - // - // [remote "origin"] - // url = git@github.com:Pix4D/cogito.git - // fetch = +refs/heads/*:refs/remotes/origin/* - // - const section = `remote "origin"` - const key = "url" - url := cfg.StringFromSection(section, key, "") - if url == "" { - return fmt.Errorf(".git/config: key [%s]/%s: not found", section, key) - } - gu, err := parseGitPseudoURL(url) - if err != nil { - return fmt.Errorf(".git/config: remote: %w", err) - } - left := []string{"github.com", owner, repo} - right := []string{gu.URL.Host, gu.Owner, gu.Repo} - for i, l := range left { - r := right[i] - if !strings.EqualFold(l, r) { - return fmt.Errorf(`the received git repository is incompatible with the Cogito configuration. - -Git repository configuration (received as 'inputs:' in this PUT step): - url: %s - owner: %s - repo: %s - -Cogito SOURCE configuration: - owner: %s - repo: %s`, - url, gu.Owner, gu.Repo, - owner, repo) - } - } - return nil -} - -type gitURL struct { - URL *url.URL - Owner string - Repo string -} - -// parseGitPseudoURL attempts to parse rawURL as a git remote URL compatible with the -// Github naming conventions. -// -// It supports the following types of git pseudo URLs: -// - ssh: git@github.com:Pix4D/cogito.git; will be rewritten to the valid URL -// ssh://git@github.com/Pix4D/cogito.git -// - https: https://github.com/Pix4D/cogito.git -// - http: http://github.com/Pix4D/cogito.git -func parseGitPseudoURL(rawURL string) (gitURL, error) { - workURL := rawURL - // If ssh pseudo URL, we need to massage the rawURL ourselves :-( - if strings.HasPrefix(workURL, "git@") { - if strings.Count(workURL, ":") != 1 { - return gitURL{}, fmt.Errorf("invalid git SSH URL %s: want exactly one ':'", rawURL) - } - // Make the URL a real URL, ready to be parsed. For example: - // git@github.com:Pix4D/cogito.git -> ssh://git@github.com/Pix4D/cogito.git - workURL = "ssh://" + strings.Replace(workURL, ":", "/", 1) - } - - anyUrl, err := url.Parse(workURL) - if err != nil { - return gitURL{}, err - } - - scheme := anyUrl.Scheme - if scheme == "" { - return gitURL{}, fmt.Errorf("invalid git URL %s: missing scheme", rawURL) - } - if scheme != "ssh" && scheme != "http" && scheme != "https" { - return gitURL{}, fmt.Errorf("invalid git URL %s: invalid scheme: %s", rawURL, scheme) - } - - // Further parse the path component of the URL to see if it complies with the Github - // naming conventions. - // Example of compliant path: github.com/Pix4D/cogito.git - tokens := strings.Split(anyUrl.Path, "/") - if have, want := len(tokens), 3; have != want { - return gitURL{}, - fmt.Errorf("invalid git URL: path: want: %d components; have: %d %s", - want, have, tokens) - } - - // All OK. Fill our gitURL struct - gu := gitURL{ - URL: anyUrl, - Owner: tokens[1], - Repo: strings.TrimSuffix(tokens[2], ".git"), - } - return gu, nil -} - -// GitGetCommit looks into a git repository and extracts the commit SHA of the HEAD. -func GitGetCommit(repoPath string) (string, error) { +// getGitCommit looks into a git repository and extracts the commit SHA of the HEAD. +func getGitCommit(repoPath string) (string, error) { dotGitPath := filepath.Join(repoPath, ".git") headPath := filepath.Join(dotGitPath, "HEAD") diff --git a/resource/resource_test.go b/resource/resource_test.go index 5f6242c7..ed84f0dd 100644 --- a/resource/resource_test.go +++ b/resource/resource_test.go @@ -262,129 +262,6 @@ Cogito SOURCE configuration: } } -func TestCheckRepoDirSuccess(t *testing.T) { - const wantOwner = "smiling" - const wantRepo = "butterfly" - - testCases := []struct { - name string - dir string // repoURL to put in file /.git/config - repoURL string - }{ - { - name: "repo with good SSH remote", - dir: "a-repo", - repoURL: sshRemote(wantOwner, wantRepo), - }, - { - name: "repo with good HTTPS remote", - dir: "a-repo", - repoURL: httpsRemote(wantOwner, wantRepo), - }, - { - name: "repo with good HTTP remote", - dir: "a-repo", - repoURL: httpRemote(wantOwner, wantRepo), - }, - { - name: "PR resource but with basic auth in URL (see PR #46)", - dir: "a-repo", - repoURL: "https://x-oauth-basic:ghp_XXX@github.com/smiling/butterfly.git", - }, - } - - for _, tc := range testCases { - inDir := setup(t, tc.dir, tc.repoURL, "dummySHA", "dummyHead") - - t.Run(tc.name, func(t *testing.T) { - err := checkRepoDir(filepath.Join(inDir, tc.dir), wantOwner, wantRepo) - - if err != nil { - t.Fatalf("\nhave: %s\nwant: ", err) - } - }) - } -} - -func TestCheckRepoDirFailure(t *testing.T) { - const wantOwner = "smiling" - const wantRepo = "butterfly" - - testCases := []struct { - name string - dir string - repoURL string // repoURL to put in file /.git/config - wantErrWild string // wildcard matching - }{ - { - name: "dir is not a repo", - dir: "not-a-repo", - repoURL: "dummyurl", - wantErrWild: `parsing .git/config: open */not-a-repo/.git/config: no such file or directory`, - }, - { - name: "bad file .git/config", - dir: "repo-bad-git-config", - repoURL: "dummyurl", - wantErrWild: `.git/config: key [remote "origin"]/url: not found`, - }, - { - name: "repo with unrelated HTTPS remote", - dir: "a-repo", - repoURL: httpsRemote("owner-a", "repo-a"), - wantErrWild: `the received git repository is incompatible with the Cogito configuration. - -Git repository configuration (received as 'inputs:' in this PUT step): - url: https://github.com/owner-a/repo-a.git - owner: owner-a - repo: repo-a - -Cogito SOURCE configuration: - owner: smiling - repo: butterfly`, - }, - { - name: "repo with unrelated SSH remote or wrong source config", - dir: "a-repo", - repoURL: sshRemote("owner-a", "repo-a"), - wantErrWild: `the received git repository is incompatible with the Cogito configuration. - -Git repository configuration (received as 'inputs:' in this PUT step): - url: git@github.com:owner-a/repo-a.git - owner: owner-a - repo: repo-a - -Cogito SOURCE configuration: - owner: smiling - repo: butterfly`, - }, - { - name: "invalid git pseudo URL in .git/config", - dir: "a-repo", - repoURL: "foo://bar", - wantErrWild: `.git/config: remote: invalid git URL foo://bar: invalid scheme: foo`, - }, - } - - for _, tc := range testCases { - inDir := setup(t, tc.dir, tc.repoURL, "dummySHA", "dummyHead") - - t.Run(tc.name, func(t *testing.T) { - err := checkRepoDir(filepath.Join(inDir, tc.dir), wantOwner, wantRepo) - - if err == nil { - t.Fatalf("\nhave: \nwant: %s", tc.wantErrWild) - } - - have := err.Error() - if !wild.Match(tc.wantErrWild, have, false) { - diff := cmp.Diff(tc.wantErrWild, have) - t.Fatalf("error msg wildcard mismatch: (-want +have):\n%s", diff) - } - }) - } -} - func TestGitGetCommitSuccess(t *testing.T) { const wantSHA = "af6cd86e98eb1485f04d38b78d9532e916bbff02" const defHead = "ref: refs/heads/a-branch-FIXME" @@ -469,164 +346,3 @@ func TestGitGetCommitFailure(t *testing.T) { }) } } - -// setup creates a temporary directory by rendering the templated contents of dir -// (assumed to be below testdata) with values from the remaining arguments and returns -// the path to the directory. -// The temporary directory is registered for removal via t.Cleanup. -// If any operation fails, setup terminates the test by calling t.Fatal. -func setup( - t *testing.T, - testDir string, - repoURL string, - commitSHA string, - head string, -) string { - inDir, err := os.MkdirTemp("", "cogito-test-") - if err != nil { - t.Fatal("setup: MkdirTemp", err) - } - - t.Cleanup(func() { - if err := os.RemoveAll(inDir); err != nil { - t.Fatal("setup: cleanup: RemoveAll:", err) - } - }) - - // Prepare the template data. - tdata := make(help.TemplateData) - tdata["repo_url"] = repoURL - tdata["commit_sha"] = commitSHA - tdata["head"] = head - tdata["branch_name"] = "a-branch-FIXME" - - // Copy the testdata over - err = help.CopyDir(inDir, filepath.Join("testdata", testDir), help.DotRenamer, tdata) - if err != nil { - t.Fatal("CopyDir:", err) - } - - return inDir -} - -// sshRemote returns a github SSH URL -func sshRemote(owner, repo string) string { - return fmt.Sprintf("git@github.com:%s/%s.git", owner, repo) -} - -// httpsRemote returns a github HTTPS URL -func httpsRemote(owner, repo string) string { - return fmt.Sprintf("https://github.com/%s/%s.git", owner, repo) -} - -// httpRemote returns a github HTTP URL -func httpRemote(owner, repo string) string { - return fmt.Sprintf("http://github.com/%s/%s.git", owner, repo) -} - -func TestParseGitPseudoURLSuccess(t *testing.T) { - testCases := []struct { - name string - inURL string - wantGU gitURL - }{ - { - name: "valid SSH URL", - inURL: "git@github.com:Pix4D/cogito.git", - wantGU: gitURL{ - URL: &url.URL{ - Scheme: "ssh", - User: url.User("git"), - Host: "github.com", - Path: "/Pix4D/cogito.git", - }, - Owner: "Pix4D", - Repo: "cogito", - }, - }, - { - name: "valid HTTPS URL", - inURL: "https://github.com/Pix4D/cogito.git", - wantGU: gitURL{ - URL: &url.URL{ - Scheme: "https", - Host: "github.com", - Path: "/Pix4D/cogito.git", - }, - Owner: "Pix4D", - Repo: "cogito", - }, - }, - { - name: "valid HTTP URL", - inURL: "http://github.com/Pix4D/cogito.git", - wantGU: gitURL{ - URL: &url.URL{ - Scheme: "http", - Host: "github.com", - Path: "/Pix4D/cogito.git", - }, - Owner: "Pix4D", - Repo: "cogito", - }, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - gitUrl, err := parseGitPseudoURL(tc.inURL) - - if err != nil { - t.Fatalf("\nhave: %s\nwant: ", err) - } - if diff := cmp.Diff(tc.wantGU, gitUrl, cmp.Comparer( - func(x, y *url.Userinfo) bool { - return x.String() == y.String() - })); diff != "" { - t.Errorf("gitURL: (-want +have):\n%s", diff) - } - }) - } -} - -func TestParseGitPseudoURLFailure(t *testing.T) { - testCases := []struct { - name string - inURL string - wantErr string - }{ - { - name: "totally invalid URL", - inURL: "hello", - wantErr: "invalid git URL hello: missing scheme", - }, - { - name: "invalid SSH URL", - inURL: "git@github.com/Pix4D/cogito.git", - wantErr: "invalid git SSH URL git@github.com/Pix4D/cogito.git: want exactly one ':'", - }, - { - name: "invalid HTTPS URL", - inURL: "https://github.com:Pix4D/cogito.git", - wantErr: `parse "https://github.com:Pix4D/cogito.git": invalid port ":Pix4D" after host`, - }, - { - name: "invalid HTTP URL", - inURL: "http://github.com:Pix4D/cogito.git", - wantErr: `parse "http://github.com:Pix4D/cogito.git": invalid port ":Pix4D" after host`, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - _, err := parseGitPseudoURL(tc.inURL) - - if err == nil { - t.Fatalf("have: ; want: %v", tc.wantErr) - } - if diff := cmp.Diff(tc.wantErr, err.Error()); diff != "" { - t.Errorf("error message mismatch: (-want +have):\n%s", diff) - } - }) - } -} diff --git a/help/testhelper.go b/testhelp/testhelper.go similarity index 72% rename from help/testhelper.go rename to testhelp/testhelper.go index 022943df..83fa2b90 100644 --- a/help/testhelper.go +++ b/testhelp/testhelper.go @@ -1,4 +1,4 @@ -package help +package testhelp import ( "bytes" @@ -27,7 +27,8 @@ func IdentityRenamer(name string) string { return name } -// Recursive copy src directory below dst directory, with optional transformations. +// CopyDir recursively copies src directory below dst directory, with optional +// transformations. // It performs the following transformations: // - Renames any directory with renamer. // - If templatedata is not empty, will consider each file ending with ".template" as a Go @@ -187,3 +188,63 @@ func MergeMap(a, b map[string]any) map[string]any { } return c } + +// MakeGitRepoFromTestdata creates a temporary directory by rendering the templated +// contents of testdataDir with values from (repoURL, commitSHA, head) and returns the +// path to the directory. +// +// MakeGitRepoFromTestdata also renames directories of the form 'dot.git' to '.git', +// thus making said directory a git repository. This allows to supply the 'dot.git' +// directory as test input, avoiding the problem of having this testdata .git directory +// a nested repository in the project repository. +// +// The temporary directory is registered for removal via t.Cleanup. +// If any operation fails, makeGitRepoFromTestdata terminates the test by calling t.Fatal. +func MakeGitRepoFromTestdata( + t *testing.T, + testdataDir string, + repoURL string, + commitSHA string, + head string, +) string { + t.Helper() + dstDir, err := os.MkdirTemp("", "cogito-test-") + if err != nil { + t.Fatal("makeGitRepoFromTestdata: MkdirTemp", err) + } + + t.Cleanup(func() { + if err := os.RemoveAll(dstDir); err != nil { + t.Fatal("makeGitRepoFromTestdata: cleanup: RemoveAll:", err) + } + }) + + // Prepare the template data. + tdata := make(TemplateData) + tdata["repo_url"] = repoURL + tdata["commit_sha"] = commitSHA + tdata["head"] = head + tdata["branch_name"] = "a-branch-FIXME" + + err = CopyDir(dstDir, testdataDir, DotRenamer, tdata) + if err != nil { + t.Fatal("CopyDir:", err) + } + + return dstDir +} + +// SshRemote returns a GitHub SSH URL +func SshRemote(owner, repo string) string { + return fmt.Sprintf("git@github.com:%s/%s.git", owner, repo) +} + +// HttpsRemote returns a GitHub HTTPS URL +func HttpsRemote(owner, repo string) string { + return fmt.Sprintf("https://github.com/%s/%s.git", owner, repo) +} + +// HttpRemote returns a GitHub HTTP URL +func HttpRemote(owner, repo string) string { + return fmt.Sprintf("http://github.com/%s/%s.git", owner, repo) +} From 5af895375c91753f2a606410cff5428774046c2e Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Wed, 27 Jul 2022 11:30:40 +0200 Subject: [PATCH 7/8] put step: rename validateInputDir to processInputDir We now perform more logic in processInputDir() instead of doing it directly in Put(). Since tests involve the file system, this allows to keep the tests for Put() slightly simpler and higher-level. PCI-2587 --- cogito/put.go | 8 ++++---- cogito/put_private_test.go | 4 ++-- cogito/put_test.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cogito/put.go b/cogito/put.go index 79a7312f..998907c2 100644 --- a/cogito/put.go +++ b/cogito/put.go @@ -64,8 +64,8 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { } log.Debug("", "state", buildState) - if err := validateInputDir(inputDir, pi.Source.Owner, pi.Source.Repo); err != nil { - return fmt.Errorf("put: validating the input dir: %s", err) + if err := processInputDir(inputDir, pi.Source.Owner, pi.Source.Repo); err != nil { + return fmt.Errorf("put: processing the input dir: %s", err) } // Following the protocol for put, we return the version and metadata. @@ -83,9 +83,9 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { return nil } -// validateInputDir checks whether dir, containing the "put inputs", conforms +// processInputDir checks whether dir, containing the "put inputs", conforms // to what we expect. -func validateInputDir(inputDir string, owner string, repo string) error { +func processInputDir(inputDir string, owner string, repo string) error { inputDirs, err := collectInputDirs(inputDir) if err != nil { return err diff --git a/cogito/put_private_test.go b/cogito/put_private_test.go index 95c0675e..4d7acbd9 100644 --- a/cogito/put_private_test.go +++ b/cogito/put_private_test.go @@ -13,7 +13,7 @@ import ( "gotest.tools/v3/assert" ) -func TestValidateInputDirFailure(t *testing.T) { +func TestProcessInputDirFailure(t *testing.T) { type testCase struct { name string inputDir string @@ -24,7 +24,7 @@ func TestValidateInputDirFailure(t *testing.T) { tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.inputDir, "https://github.com/foo", "dummySHA", "dummyHead") - err := validateInputDir(filepath.Join(tmpDir, filepath.Base(tc.inputDir)), + err := processInputDir(filepath.Join(tmpDir, filepath.Base(tc.inputDir)), "dummy-owner", "dummy-repo") assert.ErrorContains(t, err, tc.wantErr) diff --git a/cogito/put_test.go b/cogito/put_test.go index 2572c45c..41df2ec9 100644 --- a/cogito/put_test.go +++ b/cogito/put_test.go @@ -152,7 +152,7 @@ func TestPutProtocolFailure(t *testing.T) { { name: "wrong input directory from arguments", args: []string{"non-existing"}, - wantErr: "put: validating the input dir: collecting directories in non-existing: open non-existing: no such file or directory", + wantErr: "put: processing the input dir: collecting directories in non-existing: open non-existing: no such file or directory", }, } From ccc4876f2fa19e80db0e25489c2bf67536418c6c Mon Sep 17 00:00:00 2001 From: Marco Molteni Date: Wed, 27 Jul 2022 10:53:05 +0200 Subject: [PATCH 8/8] put step: input dir: process git repo PCI-2587 --- Taskfile.yml | 2 +- cmd/cogito/main_test.go | 2 +- cogito/put.go | 65 +++++- cogito/put_private_test.go | 221 +++++++++++++----- cogito/put_test.go | 20 +- .../a-repo/dot.git/HEAD.template | 0 .../a-repo/dot.git/config.template | 0 .../refs/heads/{{.branch_name}}.template | 0 resource/resource.go | 55 ----- resource/resource_test.go | 88 ------- 10 files changed, 223 insertions(+), 230 deletions(-) rename cogito/testdata/{ => one-repo}/a-repo/dot.git/HEAD.template (100%) rename cogito/testdata/{ => one-repo}/a-repo/dot.git/config.template (100%) rename cogito/testdata/{ => one-repo}/a-repo/dot.git/refs/heads/{{.branch_name}}.template (100%) diff --git a/Taskfile.yml b/Taskfile.yml index 2ca7d24c..2217ddcf 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -86,7 +86,7 @@ tasks: - rm -rf /tmp/cogito-test - mkdir -p /tmp/cogito-test - > - ./bin/copydir cogito/testdata/a-repo /tmp/cogito-test --dot + ./bin/copydir cogito/testdata/one-repo/a-repo /tmp/cogito-test --dot --template repo_url=https://github.com/foo/bar head=dummyHead branch_name=dummyBranch commit_sha=dummySha - echo '{{.PUT_INPUT}}' | ./bin/out /tmp/cogito-test diff --git a/cmd/cogito/main_test.go b/cmd/cogito/main_test.go index f222fa10..fd1b1f50 100644 --- a/cmd/cogito/main_test.go +++ b/cmd/cogito/main_test.go @@ -60,7 +60,7 @@ func TestRunPutSmokeSuccess(t *testing.T) { }`) var out bytes.Buffer var logOut bytes.Buffer - inputDir := testhelp.MakeGitRepoFromTestdata(t, "../../cogito/testdata/a-repo", + inputDir := testhelp.MakeGitRepoFromTestdata(t, "../../cogito/testdata/one-repo/a-repo", testhelp.HttpsRemote("the-owner", "the-repo"), "dummySHA", "dummyHead") err := run(in, &out, &logOut, []string{"out", inputDir}) diff --git a/cogito/put.go b/cogito/put.go index 998907c2..f24d4940 100644 --- a/cogito/put.go +++ b/cogito/put.go @@ -64,9 +64,11 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { } log.Debug("", "state", buildState) - if err := processInputDir(inputDir, pi.Source.Owner, pi.Source.Repo); err != nil { + gitHash, err := processInputDir(inputDir, pi.Source.Owner, pi.Source.Repo) + if err != nil { return fmt.Errorf("put: processing the input dir: %s", err) } + log.Debug("", "git-commit", gitHash) // Following the protocol for put, we return the version and metadata. // For Cogito, the metadata contains the Concourse build state. @@ -83,25 +85,32 @@ func Put(log hclog.Logger, in io.Reader, out io.Writer, args []string) error { return nil } -// processInputDir checks whether dir, containing the "put inputs", conforms -// to what we expect. -func processInputDir(inputDir string, owner string, repo string) error { +// processInputDir checks whether inputDir, containing the "put inputs", conforms to +// what we expect and returns the git commit hash of the repo passed as put input. +func processInputDir(inputDir string, owner string, repo string) (string, error) { inputDirs, err := collectInputDirs(inputDir) if err != nil { - return err + return "", err } if len(inputDirs) != 1 { - return fmt.Errorf( + return "", fmt.Errorf( "found %d input dirs: %v. Want exactly 1, corresponding to the GitHub repo %s/%s", len(inputDirs), inputDirs, owner, repo) } + // Since we require inputDir to contain only one directory, we assume that this + // directory is the git repo. repoDir := filepath.Join(inputDir, inputDirs[0]) if err := checkGitRepoDir(repoDir, owner, repo); err != nil { - return err + return "", err } - return nil + gitHash, err := getGitCommit(repoDir) + if err != nil { + return "", err + } + + return gitHash, nil } // collectInputDirs returns a list of all directories below dir (non-recursive). @@ -226,3 +235,43 @@ func parseGitPseudoURL(rawURL string) (gitURL, error) { } return gu, nil } + +// getGitCommit looks into a git repository and extracts the commit SHA of the HEAD. +func getGitCommit(repoPath string) (string, error) { + dotGitPath := filepath.Join(repoPath, ".git") + + headPath := filepath.Join(dotGitPath, "HEAD") + headBuf, err := os.ReadFile(headPath) + if err != nil { + return "", fmt.Errorf("git commit: read HEAD: %w", err) + } + + // The HEAD file can have two completely different contents: + // 1. if a branch checkout: "ref: refs/heads/BRANCH_NAME" + // 2. if a detached head : the commit SHA + // A detached head with Concourse happens in two cases: + // 1. if the git resource has a `tag_filter:` + // 2. if the git resource has a `version:` + + head := strings.TrimSuffix(string(headBuf), "\n") + tokens := strings.Fields(head) + var sha string + switch len(tokens) { + case 1: + // detached head + sha = head + case 2: + // branch checkout + shaRelPath := tokens[1] + shaPath := filepath.Join(dotGitPath, shaRelPath) + shaBuf, err := os.ReadFile(shaPath) + if err != nil { + return "", fmt.Errorf("git commit: branch checkout: read SHA file: %w", err) + } + sha = strings.TrimSuffix(string(shaBuf), "\n") + default: + return "", fmt.Errorf("git commit: invalid HEAD format: %q", head) + } + + return sha, nil +} diff --git a/cogito/put_private_test.go b/cogito/put_private_test.go index 4d7acbd9..51a7b092 100644 --- a/cogito/put_private_test.go +++ b/cogito/put_private_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/Pix4D/cogito/testhelp" - "github.com/gertd/wild" "github.com/google/go-cmp/cmp" "gotest.tools/v3/assert" ) @@ -22,9 +21,9 @@ func TestProcessInputDirFailure(t *testing.T) { test := func(t *testing.T, tc testCase) { tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.inputDir, - "https://github.com/foo", "dummySHA", "dummyHead") + "https://github.com/dummy-owner/dummy-repo", "dummySHA", "banana mango") - err := processInputDir(filepath.Join(tmpDir, filepath.Base(tc.inputDir)), + _, err := processInputDir(filepath.Join(tmpDir, filepath.Base(tc.inputDir)), "dummy-owner", "dummy-repo") assert.ErrorContains(t, err, tc.wantErr) @@ -41,23 +40,38 @@ func TestProcessInputDirFailure(t *testing.T) { inputDir: "testdata/not-a-repo", wantErr: "parsing .git/config: open ", }, + { + name: "git repo, but something wrong", + inputDir: "testdata/one-repo", + wantErr: "git commit: branch checkout: read SHA file: open ", + }, } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - test(t, tc) - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } - } func TestCollectInputDirs(t *testing.T) { - var testCases = []struct { + type testCase = struct { name string dir string wantErr error wantN int - }{ + } + + test := func(t *testing.T, tc testCase) { + dirs, err := collectInputDirs(tc.dir) + if !errors.Is(err, tc.wantErr) { + t.Errorf("sut(%v): error: have %v; want %v", tc.dir, err, tc.wantErr) + } + gotN := len(dirs) + if gotN != tc.wantN { + t.Errorf("sut(%v): len(dirs): have %v; want %v", tc.dir, gotN, tc.wantN) + } + } + + var testCases = []testCase{ { name: "non existing base directory", dir: "non-existing", @@ -79,87 +93,95 @@ func TestCollectInputDirs(t *testing.T) { } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - dirs, err := collectInputDirs(tc.dir) - if !errors.Is(err, tc.wantErr) { - t.Errorf("sut(%v): error: have %v; want %v", tc.dir, err, tc.wantErr) - } - gotN := len(dirs) - if gotN != tc.wantN { - t.Errorf("sut(%v): len(dirs): have %v; want %v", tc.dir, gotN, tc.wantN) - } - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } func TestCheckGitRepoDirSuccess(t *testing.T) { - const wantOwner = "smiling" - const wantRepo = "butterfly" - - testCases := []struct { + type testCase struct { name string dir string // repoURL to put in file /.git/config repoURL string - }{ + } + + const wantOwner = "smiling" + const wantRepo = "butterfly" + + test := func(t *testing.T, tc testCase) { + inputDir := testhelp.MakeGitRepoFromTestdata(t, tc.dir, tc.repoURL, + "dummySHA", "dummyHead") + + err := checkGitRepoDir(filepath.Join(inputDir, filepath.Base(tc.dir)), + wantOwner, wantRepo) + + assert.NilError(t, err) + } + + testCases := []testCase{ { name: "repo with good SSH remote", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: testhelp.SshRemote(wantOwner, wantRepo), }, { name: "repo with good HTTPS remote", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: testhelp.HttpsRemote(wantOwner, wantRepo), }, { name: "repo with good HTTP remote", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: testhelp.HttpRemote(wantOwner, wantRepo), }, { name: "PR resource but with basic auth in URL (see PR #46)", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: "https://x-oauth-basic:ghp_XXX@github.com/smiling/butterfly.git", }, } for _, tc := range testCases { - inputDir := testhelp.MakeGitRepoFromTestdata(t, filepath.Join("testdata", tc.dir), - tc.repoURL, "dummySHA", "dummyHead") - - t.Run(tc.name, func(t *testing.T) { - err := checkGitRepoDir(filepath.Join(inputDir, tc.dir), wantOwner, wantRepo) - - assert.NilError(t, err) - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } func TestCheckGitRepoDirFailure(t *testing.T) { - const wantOwner = "smiling" - const wantRepo = "butterfly" - - testCases := []struct { + type testCase struct { name string dir string repoURL string // repoURL to put in file /.git/config wantErrWild string // wildcard matching - }{ + } + + const wantOwner = "smiling" + const wantRepo = "butterfly" + + test := func(t *testing.T, tc testCase) { + inDir := testhelp.MakeGitRepoFromTestdata(t, tc.dir, tc.repoURL, + "dummySHA", "dummyHead") + + err := checkGitRepoDir(filepath.Join(inDir, filepath.Base(tc.dir)), + wantOwner, wantRepo) + + assert.ErrorContains(t, err, tc.wantErrWild) + } + + testCases := []testCase{ { name: "dir is not a repo", - dir: "not-a-repo", + dir: "testdata/not-a-repo", repoURL: "dummyurl", - wantErrWild: `parsing .git/config: open */not-a-repo/.git/config: no such file or directory`, + wantErrWild: "parsing .git/config: open ", }, { name: "bad file .git/config", - dir: "repo-bad-git-config", + dir: "testdata/repo-bad-git-config", repoURL: "dummyurl", wantErrWild: `.git/config: key [remote "origin"]/url: not found`, }, { name: "repo with unrelated HTTPS remote", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: testhelp.HttpsRemote("owner-a", "repo-a"), wantErrWild: `the received git repository is incompatible with the Cogito configuration. @@ -174,7 +196,7 @@ Cogito SOURCE configuration: }, { name: "repo with unrelated SSH remote or wrong source config", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: testhelp.SshRemote("owner-a", "repo-a"), wantErrWild: `the received git repository is incompatible with the Cogito configuration. @@ -189,29 +211,14 @@ Cogito SOURCE configuration: }, { name: "invalid git pseudo URL in .git/config", - dir: "a-repo", + dir: "testdata/one-repo/a-repo", repoURL: "foo://bar", wantErrWild: `.git/config: remote: invalid git URL foo://bar: invalid scheme: foo`, }, } for _, tc := range testCases { - inDir := testhelp.MakeGitRepoFromTestdata(t, filepath.Join("testdata", tc.dir), - tc.repoURL, "dummySHA", "dummyHead") - - t.Run(tc.name, func(t *testing.T) { - err := checkGitRepoDir(filepath.Join(inDir, tc.dir), wantOwner, wantRepo) - - if err == nil { - t.Fatalf("\nhave: \nwant: %s", tc.wantErrWild) - } - - have := err.Error() - if !wild.Match(tc.wantErrWild, have, false) { - diff := cmp.Diff(tc.wantErrWild, have) - t.Fatalf("error msg wildcard mismatch: (-want +have):\n%s", diff) - } - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } @@ -326,3 +333,91 @@ func TestParseGitPseudoURLFailure(t *testing.T) { }) } } + +func TestGitGetCommitSuccess(t *testing.T) { + type testCase struct { + name string + dir string + repoURL string + head string + } + + const wantSHA = "af6cd86e98eb1485f04d38b78d9532e916bbff02" + const defHead = "ref: refs/heads/a-branch-FIXME" + + test := func(t *testing.T, tc testCase) { + tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.dir, tc.repoURL, wantSHA, tc.head) + + sha, err := getGitCommit(filepath.Join(tmpDir, filepath.Base(tc.dir))) + + assert.NilError(t, err) + assert.Equal(t, sha, wantSHA) + } + + testCases := []testCase{ + { + name: "happy path for branch checkout", + dir: "testdata/one-repo/a-repo", + repoURL: "dummy", + head: defHead, + }, + { + name: "happy path for detached HEAD checkout", + dir: "testdata/one-repo/a-repo", + repoURL: "dummy", + head: wantSHA, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) + } +} + +func TestGitGetCommitFailure(t *testing.T) { + type testCase struct { + name string + dir string + repoURL string + head string + wantErr string + } + + const wantSHA = "af6cd86e98eb1485f04d38b78d9532e916bbff02" + + test := func(t *testing.T, tc testCase) { + tmpDir := testhelp.MakeGitRepoFromTestdata(t, tc.dir, tc.repoURL, wantSHA, tc.head) + + _, err := getGitCommit(filepath.Join(tmpDir, filepath.Base(tc.dir))) + + assert.ErrorContains(t, err, tc.wantErr) + } + + testCases := []testCase{ + { + name: "missing HEAD", + dir: "testdata/not-a-repo", + repoURL: "dummy", + head: "dummy", + wantErr: "git commit: read HEAD: open ", + }, + { + name: "invalid format for HEAD", + dir: "testdata/one-repo/a-repo", + repoURL: "dummyURL", + head: "this is a bad head", + wantErr: `git commit: invalid HEAD format: "this is a bad head"`, + }, + { + name: "HEAD points to non-existent file", + dir: "testdata/one-repo/a-repo", + repoURL: "dummyURL", + head: "banana mango", + wantErr: "git commit: branch checkout: read SHA file: open ", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) + } +} diff --git a/cogito/put_test.go b/cogito/put_test.go index 41df2ec9..2696691a 100644 --- a/cogito/put_test.go +++ b/cogito/put_test.go @@ -25,7 +25,7 @@ func TestPutSuccess(t *testing.T) { in := bytes.NewReader(toJSON(t, tc.in)) var out bytes.Buffer log := hclog.NewNullLogger() - inputDir := testhelp.MakeGitRepoFromTestdata(t, "testdata/a-repo", + inputDir := testhelp.MakeGitRepoFromTestdata(t, "testdata/one-repo/a-repo", testhelp.HttpsRemote("the-owner", "the-repo"), "dummySHA", "dummyHead") err := cogito.Put(log, in, &out, []string{inputDir}) @@ -59,9 +59,7 @@ func TestPutSuccess(t *testing.T) { } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - test(t, tc) - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } @@ -113,9 +111,7 @@ func TestPutPipelineValidationFailure(t *testing.T) { } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - test(t, tc) - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } @@ -157,9 +153,7 @@ func TestPutProtocolFailure(t *testing.T) { } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - test(t, tc) - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } @@ -174,7 +168,7 @@ func TestPutSystemFailure(t *testing.T) { test := func(t *testing.T, tc testCase) { assert.Assert(t, tc.wantErr != "") log := hclog.NewNullLogger() - inputDir := testhelp.MakeGitRepoFromTestdata(t, "testdata/a-repo", + inputDir := testhelp.MakeGitRepoFromTestdata(t, "testdata/one-repo/a-repo", testhelp.HttpsRemote("the-owner", "the-repo"), "dummySHA", "dummyHead") err := cogito.Put(log, tc.reader, tc.writer, []string{inputDir}) @@ -208,9 +202,7 @@ func TestPutSystemFailure(t *testing.T) { } for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - test(t, tc) - }) + t.Run(tc.name, func(t *testing.T) { test(t, tc) }) } } diff --git a/cogito/testdata/a-repo/dot.git/HEAD.template b/cogito/testdata/one-repo/a-repo/dot.git/HEAD.template similarity index 100% rename from cogito/testdata/a-repo/dot.git/HEAD.template rename to cogito/testdata/one-repo/a-repo/dot.git/HEAD.template diff --git a/cogito/testdata/a-repo/dot.git/config.template b/cogito/testdata/one-repo/a-repo/dot.git/config.template similarity index 100% rename from cogito/testdata/a-repo/dot.git/config.template rename to cogito/testdata/one-repo/a-repo/dot.git/config.template diff --git a/cogito/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template b/cogito/testdata/one-repo/a-repo/dot.git/refs/heads/{{.branch_name}}.template similarity index 100% rename from cogito/testdata/a-repo/dot.git/refs/heads/{{.branch_name}}.template rename to cogito/testdata/one-repo/a-repo/dot.git/refs/heads/{{.branch_name}}.template diff --git a/resource/resource.go b/resource/resource.go index d8014937..d056bff4 100644 --- a/resource/resource.go +++ b/resource/resource.go @@ -5,9 +5,6 @@ package resource import ( "fmt" - "os" - "path/filepath" - "strings" "github.com/Pix4D/cogito/github" oc "github.com/cloudboss/ofcourse/ofcourse" @@ -64,12 +61,6 @@ func (r *Resource) Out( // STUFF DELETED - gitRef, err := getGitCommit(repoDir) - if err != nil { - return nil, nil, err - } - log.Debugf("out: parsed ref %q", gitRef) - // // Post the status to all sinks and collect the sinkErrors. // @@ -102,49 +93,3 @@ func (r *Resource) Out( return dummyVersion, metadata, nil } - -// getGitCommit looks into a git repository and extracts the commit SHA of the HEAD. -func getGitCommit(repoPath string) (string, error) { - dotGitPath := filepath.Join(repoPath, ".git") - - headPath := filepath.Join(dotGitPath, "HEAD") - headBuf, err := os.ReadFile(headPath) - if err != nil { - return "", fmt.Errorf("git commit: read HEAD: %w", err) - } - - // The HEAD file can have two completely different contents: - // 1. if a branch checkout: "ref: refs/heads/BRANCH_NAME" - // 2. if a detached head : the commit SHA - // A detached head with Concourse happens in two cases: - // 1. if the git resource has a `tag_filter:` - // 2. if the git resource has a `version:` - - head := strings.TrimSuffix(string(headBuf), "\n") - tokens := strings.Fields(head) - var sha string - switch len(tokens) { - case 1: - // detached head - sha = head - case 2: - // branch checkout - shaRelPath := tokens[1] - shaPath := filepath.Join(dotGitPath, shaRelPath) - shaBuf, err := os.ReadFile(shaPath) - if err != nil { - return "", fmt.Errorf("git commit: branch checkout: read SHA file: %w", err) - } - sha = strings.TrimSuffix(string(shaBuf), "\n") - default: - return "", fmt.Errorf("git commit: invalid HEAD format: %q", head) - } - - // Minimal validation that the file contents look like a 40-digit SHA. - const shaLen = 40 - if len(sha) != shaLen { - return "", fmt.Errorf("git commit: SHA %s: have len of %d; want %d", sha, len(sha), shaLen) - } - - return sha, nil -} diff --git a/resource/resource_test.go b/resource/resource_test.go index ed84f0dd..4b74953a 100644 --- a/resource/resource_test.go +++ b/resource/resource_test.go @@ -6,13 +6,10 @@ import ( "io" "net/http" "net/http/httptest" - "net/url" "os" - "path/filepath" "testing" oc "github.com/cloudboss/ofcourse/ofcourse" - "github.com/gertd/wild" "github.com/google/go-cmp/cmp" "github.com/Pix4D/cogito/help" @@ -261,88 +258,3 @@ Cogito SOURCE configuration: }) } } - -func TestGitGetCommitSuccess(t *testing.T) { - const wantSHA = "af6cd86e98eb1485f04d38b78d9532e916bbff02" - const defHead = "ref: refs/heads/a-branch-FIXME" - - testCases := []struct { - name string - dir string - repoURL string - head string - }{ - { - name: "happy path for branch checkout", - dir: "a-repo", - repoURL: "dummy", - head: defHead, - }, - { - name: "happy path for detached HEAD checkout", - dir: "a-repo", - repoURL: "dummy", - head: wantSHA, - }, - } - - for _, tc := range testCases { - dir := setup(t, tc.dir, tc.repoURL, wantSHA, tc.head) - - t.Run(tc.name, func(t *testing.T) { - sha, err := GitGetCommit(filepath.Join(dir, tc.dir)) - - if err != nil { - t.Fatalf("\nhave: %s\nwant: ", err) - } - if sha != wantSHA { - t.Fatalf("ref: have: %s; want: %s", sha, wantSHA) - } - }) - } -} - -func TestGitGetCommitFailure(t *testing.T) { - const wantSHA = "af6cd86e98eb1485f04d38b78d9532e916bbff02" - - testCases := []struct { - name string - dir string - repoURL string - head string - wantErrWild string // wildcard matching - }{ - { - name: "missing HEAD", - dir: "not-a-repo", - repoURL: "dummy", - head: "dummy", - wantErrWild: `git commit: read HEAD: open */not-a-repo/.git/HEAD: no such file or directory`, - }, - { - name: "invalid format for HEAD", - dir: "a-repo", - repoURL: "dummyURL", - head: "this is a bad head", - wantErrWild: `git commit: invalid HEAD format: "this is a bad head"`, - }, - } - - for _, tc := range testCases { - dir := setup(t, tc.dir, tc.repoURL, wantSHA, tc.head) - - t.Run(tc.name, func(t *testing.T) { - _, err := GitGetCommit(filepath.Join(dir, tc.dir)) - - if err == nil { - t.Fatalf("\nhave: \nwant: %s", tc.wantErrWild) - } - - have := err.Error() - if !wild.Match(tc.wantErrWild, have, false) { - diff := cmp.Diff(tc.wantErrWild, have) - t.Fatalf("error msg wildcard mismatch: (-want +have):\n%s", diff) - } - }) - } -}