Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
Shahul authored and Shahul committed Sep 22, 2021
1 parent cabaa44 commit 7e81dba
Show file tree
Hide file tree
Showing 9 changed files with 84 additions and 74 deletions.
7 changes: 3 additions & 4 deletions cmd/bundle/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import (
"github.com/google/go-containerregistry/pkg/authn"
"github.com/google/go-containerregistry/pkg/name"
"github.com/google/go-containerregistry/pkg/v1/remote"
"github.com/shipwright-io/build/pkg/bundle"
"github.com/spf13/pflag"

"github.com/shipwright-io/build/pkg/bundle"
)

var flagValues struct {
Expand Down Expand Up @@ -78,9 +79,7 @@ func Do(ctx context.Context) error {
}

if flagValues.imageDigest != "" {
if err = ioutil.WriteFile(
flagValues.imageDigest, []byte(digest.String()), 0644,
); err != nil {
if err = ioutil.WriteFile(flagValues.imageDigest, []byte(digest.String()), 0644); err != nil {
return err
}
}
Expand Down
15 changes: 8 additions & 7 deletions cmd/bundle/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ import (
"os"
"path/filepath"

"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

. "github.com/shipwright-io/build/cmd/bundle"

"github.com/google/go-containerregistry/pkg/name"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/google/go-containerregistry/pkg/v1/remote"
)

var _ = Describe("Bundle Loader", func() {
Expand All @@ -35,21 +36,21 @@ var _ = Describe("Bundle Loader", func() {
f(path)
}

withTempFile := func(pattern string, f func(filename string)) {
var withTempFile = func(pattern string, f func(filename string)) {
file, err := ioutil.TempFile(os.TempDir(), pattern)
Expect(err).ToNot(HaveOccurred())
defer os.Remove(file.Name())

f(file.Name())
}

filecontent := func(path string) string {
var filecontent = func(path string) string {
data, err := ioutil.ReadFile(path)
Expect(err).ToNot(HaveOccurred())
return string(data)
}

getImage := func(tag name.Tag) v1.Image {
var getImage = func(tag name.Tag) v1.Image {
ref, err := name.ParseReference(tag.String())
Expect(err).To(BeNil())

Expand All @@ -62,7 +63,7 @@ var _ = Describe("Bundle Loader", func() {
return img
}

getImageDigest := func(tag name.Tag) v1.Hash {
var getImageDigest = func(tag name.Tag) v1.Hash {
digest, err := getImage(tag).Digest()
Expect(err).To(BeNil())

Expand Down
4 changes: 1 addition & 3 deletions cmd/git/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,7 @@ func runGitClone(ctx context.Context) error {
return err
}

if err = ioutil.WriteFile(
flagValues.resultFileCommitAuthor, []byte(output), 0644,
); err != nil {
if err = ioutil.WriteFile(flagValues.resultFileCommitAuthor, []byte(output), 0644); err != nil {
return err
}
}
Expand Down
2 changes: 1 addition & 1 deletion cmd/git/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ var _ = Describe("Git Resource", func() {
})
})

Context("store result", func() {
Context("store details in result files", func() {
const exampleRepo = "https://github.com/shipwright-io/sample-go"

It("should store commit-sha into file specified in --result-file-commit-sha flag", func() {
Expand Down
8 changes: 4 additions & 4 deletions docs/buildrun.md
Original file line number Diff line number Diff line change
Expand Up @@ -233,10 +233,10 @@ status:
digest: sha256:07626e3c7fdd28d5328a8d6df8d29cd3da760c7f5e2070b534f9b880ed093a53
size: "1989004"
sources:
- git:
- name: default
git:
commitAuthor: xxx xxxxxx
commitSha: f25822b85021d02059c9ac8a211ef3804ea8fdde
name: default
```

Another example of a `BuildRun` with surfaced results for local source code(`bundle`) source:
Expand All @@ -250,9 +250,9 @@ status:
digest: sha256:07626e3c7fdd28d5328a8d6df8d29cd3da760c7f5e2070b534f9b880ed093a53
size: "1989004"
sources:
- bundle:
- name: default
bundle:
digest: sha256:0f5e2070b534f9b880ed093a537626e3c7fdd28d5328a8d6df8d29cd3da760c7
name: default
```

### Build Snapshot
Expand Down
6 changes: 1 addition & 5 deletions pkg/bundle/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,7 @@ func PackAndPush(ref name.Reference, directory string, options ...remote.Option)
// PullAndUnpack a container image layer content into a local directory. Analog
// to the bundle.PackAndPush function, optional remote.Option can be used to
// configure settings for the image pull, i.e. access credentials.
func PullAndUnpack(
ref name.Reference,
targetPath string,
options ...remote.Option,
) (*v1.Image, error) {
func PullAndUnpack(ref name.Reference, targetPath string, options ...remote.Option) (*v1.Image, error) {
desc, err := remote.Get(ref, options...)
if err != nil {
return nil, err
Expand Down
33 changes: 21 additions & 12 deletions pkg/reconciler/buildrun/resources/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@ package resources
import (
"fmt"

buildv1alpha1 "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
build "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"

pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

type sourceResult struct {
defaultSource,
bundleSource buildv1alpha1.SourceResult
bundleSource build.SourceResult
}

const (
Expand All @@ -28,17 +29,13 @@ const (
// UpdateBuildRunUsingTaskResults surface the task results
// to the buildrun
func UpdateBuildRunUsingTaskResults(
buildRun *buildv1alpha1.BuildRun,
lastTaskRun *v1beta1.TaskRun,
buildRun *build.BuildRun,
lastTaskRun *pipeline.TaskRun,
) {
var sources sourceResult

// Initializing source result
sources.defaultSource.Git = &buildv1alpha1.GitSourceResult{}
sources.bundleSource.Bundle = &buildv1alpha1.BundleSourceResult{}

// Initializing output result
buildRun.Status.Output = &buildv1alpha1.Output{}
buildRun.Status.Output = &build.Output{}

for _, result := range lastTaskRun.Status.TaskRunResults {
updateBuildRunStatus(buildRun, result, &sources)
Expand All @@ -56,20 +53,32 @@ func UpdateBuildRunUsingTaskResults(
}

func updateBuildRunStatus(
buildRun *buildv1alpha1.BuildRun,
result v1beta1.TaskRunResult,
buildRun *build.BuildRun,
result pipeline.TaskRunResult,
sources *sourceResult,
) {
switch result.Name {
case generateSourceResultName(defaultSourceName, commitSHAResult):
if sources.defaultSource.Git == nil {
sources.defaultSource.Git = &build.GitSourceResult{}
}

// Source name is default as `spec.source` has no name field
sources.defaultSource.Name = defaultSourceName
sources.defaultSource.Git.CommitSha = result.Value
case generateSourceResultName(defaultSourceName, commitAuthorResult):
if sources.defaultSource.Git == nil {
sources.defaultSource.Git = &build.GitSourceResult{}
}

// Source name is default as `spec.source` has no name field
sources.defaultSource.Name = defaultSourceName
sources.defaultSource.Git.CommitAuthor = result.Value
case generateSourceResultName(defaultSourceName, bundleImageDigestResult):
if sources.defaultSource.Bundle == nil {
sources.bundleSource.Bundle = &build.BundleSourceResult{}
}

// Source name is default as `spec.source` has no name field
sources.bundleSource.Name = defaultSourceName
sources.bundleSource.Bundle.Digest = result.Value
Expand Down
76 changes: 43 additions & 33 deletions pkg/reconciler/buildrun/resources/results_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ import (
. "github.com/onsi/ginkgo"
. "github.com/onsi/gomega"

"github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
build "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
"github.com/shipwright-io/build/pkg/reconciler/buildrun/resources"
"github.com/shipwright-io/build/test"

pipelinev1beta1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

Expand All @@ -19,7 +20,7 @@ var _ = Describe("TaskRun results to BuildRun", func() {

Context("when a BuildRun complete successfully", func() {
var (
br *v1alpha1.BuildRun
br *build.BuildRun
tr *pipelinev1beta1.TaskRun
)

Expand All @@ -31,13 +32,15 @@ var _ = Describe("TaskRun results to BuildRun", func() {
It("should surface the TaskRun results emitting from default(git) source step", func() {
commitSha := "0e0583421a5e4bf562ffe33f3651e16ba0c78591"

tr.Status.TaskRunResults = append(tr.Status.TaskRunResults, pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-sha",
Value: commitSha,
}, pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-author",
Value: "foo bar",
})
tr.Status.TaskRunResults = append(tr.Status.TaskRunResults,
pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-sha",
Value: commitSha,
},
pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-author",
Value: "foo bar",
})

resources.UpdateBuildRunUsingTaskResults(br, tr)

Expand All @@ -49,10 +52,11 @@ var _ = Describe("TaskRun results to BuildRun", func() {
It("should surface the TaskRun results emitting from default(bundle) source step", func() {
bundleImageDigest := "sha256:fe1b73cd25ac3f11dec752755e2"

tr.Status.TaskRunResults = append(tr.Status.TaskRunResults, pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-bundle-image-digest",
Value: bundleImageDigest,
})
tr.Status.TaskRunResults = append(tr.Status.TaskRunResults,
pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-bundle-image-digest",
Value: bundleImageDigest,
})

resources.UpdateBuildRunUsingTaskResults(br, tr)

Expand All @@ -63,13 +67,15 @@ var _ = Describe("TaskRun results to BuildRun", func() {
It("should surface the TaskRun results emitting from output step", func() {
imageDigest := "sha256:fe1b73cd25ac3f11dec752755e2"

tr.Status.TaskRunResults = append(tr.Status.TaskRunResults, pipelinev1beta1.TaskRunResult{
Name: "shp-image-digest",
Value: imageDigest,
}, pipelinev1beta1.TaskRunResult{
Name: "shp-image-size",
Value: "230",
})
tr.Status.TaskRunResults = append(tr.Status.TaskRunResults,
pipelinev1beta1.TaskRunResult{
Name: "shp-image-digest",
Value: imageDigest,
},
pipelinev1beta1.TaskRunResult{
Name: "shp-image-size",
Value: "230",
})

resources.UpdateBuildRunUsingTaskResults(br, tr)

Expand All @@ -81,19 +87,23 @@ var _ = Describe("TaskRun results to BuildRun", func() {
commitSha := "0e0583421a5e4bf562ffe33f3651e16ba0c78591"
imageDigest := "sha256:fe1b73cd25ac3f11dec752755e2"

tr.Status.TaskRunResults = append(tr.Status.TaskRunResults, pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-sha",
Value: commitSha,
}, pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-author",
Value: "foo bar",
}, pipelinev1beta1.TaskRunResult{
Name: "shp-image-digest",
Value: imageDigest,
}, pipelinev1beta1.TaskRunResult{
Name: "shp-image-size",
Value: "230",
})
tr.Status.TaskRunResults = append(tr.Status.TaskRunResults,
pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-sha",
Value: commitSha,
},
pipelinev1beta1.TaskRunResult{
Name: "shp-source-default-commit-author",
Value: "foo bar",
},
pipelinev1beta1.TaskRunResult{
Name: "shp-image-digest",
Value: imageDigest,
},
pipelinev1beta1.TaskRunResult{
Name: "shp-image-size",
Value: "230",
})

resources.UpdateBuildRunUsingTaskResults(br, tr)

Expand Down
7 changes: 2 additions & 5 deletions pkg/reconciler/buildrun/resources/sources/bundle.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (

build "github.com/shipwright-io/build/pkg/apis/build/v1alpha1"
"github.com/shipwright-io/build/pkg/config"

pipeline "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

Expand All @@ -37,11 +38,7 @@ func AppendBundleStep(
bundleStep.Container.Args = []string{
"--image", source.BundleContainer.Image,
"--target", fmt.Sprintf("$(params.%s-%s)", prefixParamsResultsVolumes, paramSourceRoot),
"--result-file-image-digest",
fmt.Sprintf(
"$(results.%s-source-%s-bundle-image-digest.path)",
prefixParamsResultsVolumes, name,
),
"--result-file-image-digest", fmt.Sprintf("$(results.%s-source-%s-bundle-image-digest.path)", prefixParamsResultsVolumes, name),
}

// add credentials mount, if provided
Expand Down

0 comments on commit 7e81dba

Please sign in to comment.