Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for Manifest annotations in bundle push #2105

Merged
merged 1 commit into from
Aug 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/cmd/tkn_bundle_push.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ Input:
### Options

```
--annotate strings OCI Manifest annotation in the form of key=value to be added to the OCI image. Can be provided multiple times to add multiple annotations.
-f, --filenames strings List of fully-qualified file paths containing YAML or JSON defined Tekton objects to include in this bundle
-h, --help help for push
--remote-bearer string A Bearer token to authenticate against the repository
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/tkn-bundle-push.1
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ Input:


.SH OPTIONS
.PP
\fB\-\-annotate\fP=[]
OCI Manifest annotation in the form of key=value to be added to the OCI image. Can be provided multiple times to add multiple annotations.

.PP
\fB\-f\fP, \fB\-\-filenames\fP=[]
List of fully\-qualified file paths containing YAML or JSON defined Tekton objects to include in this bundle
Expand Down
4 changes: 2 additions & 2 deletions pkg/bundle/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (

// BuildTektonBundle will return a complete OCI Image usable as a Tekton Bundle built by parsing, decoding, and
// compressing the provided contents as Tekton objects.
func BuildTektonBundle(contents []string, log io.Writer) (v1.Image, error) {
img := empty.Image
func BuildTektonBundle(contents []string, annotations map[string]string, log io.Writer) (v1.Image, error) {
img := mutate.Annotations(empty.Image, annotations).(v1.Image)

if len(contents) > tkremote.MaximumBundleObjects {
return nil, fmt.Errorf("bundle contains more than the maximum %d allow objects", tkremote.MaximumBundleObjects)
Expand Down
16 changes: 11 additions & 5 deletions pkg/bundle/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ func TestBuildTektonBundle(t *testing.T) {
return
}

img, err := BuildTektonBundle([]string{string(raw)}, &bytes.Buffer{})
annotations := map[string]string{"org.opencontainers.image.license": "Apache-2.0", "org.opencontainers.image.url": "https://example.org"}
img, err := BuildTektonBundle([]string{string(raw)}, annotations, &bytes.Buffer{})
if err != nil {
t.Error(err)
}
Expand All @@ -54,6 +55,11 @@ func TestBuildTektonBundle(t *testing.T) {
return
}

ann := manifest.Annotations
if len(ann) != len(annotations) || fmt.Sprint(ann) != fmt.Sprint(annotations) {
t.Errorf("Requested annotations were not set wanted: %s, got %s", annotations, ann)
}

if len(manifest.Layers) != 1 {
t.Errorf("Unexpected number of layers %d", len(manifest.Layers))
}
Expand Down Expand Up @@ -123,7 +129,7 @@ func TestBadObj(t *testing.T) {
t.Error(err)
return
}
_, err = BuildTektonBundle([]string{string(raw)}, &bytes.Buffer{})
_, err = BuildTektonBundle([]string{string(raw)}, nil, &bytes.Buffer{})
noNameErr := errors.New("kubernetes resources should have a name")
if err == nil {
t.Errorf("expected error: %v", noNameErr)
Expand All @@ -146,7 +152,7 @@ func TestLessThenMaxBundle(t *testing.T) {
return
}
// no error for less then max
_, err = BuildTektonBundle([]string{string(raw)}, &bytes.Buffer{})
_, err = BuildTektonBundle([]string{string(raw)}, nil, &bytes.Buffer{})
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -174,7 +180,7 @@ func TestJustEnoughBundleSize(t *testing.T) {
justEnoughObj = append(justEnoughObj, string(raw))
}
// no error for the max
_, err := BuildTektonBundle(justEnoughObj, &bytes.Buffer{})
_, err := BuildTektonBundle(justEnoughObj, nil, &bytes.Buffer{})
if err != nil {
t.Error(err)
}
Expand Down Expand Up @@ -203,7 +209,7 @@ func TestTooManyInBundle(t *testing.T) {
}

// expect error when we hit the max
_, err := BuildTektonBundle(toMuchObj, &bytes.Buffer{})
_, err := BuildTektonBundle(toMuchObj, nil, &bytes.Buffer{})
if err == nil {
t.Errorf("expected error: %v", toManyObjErr)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/cmd/bundle/list_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestListCommand(t *testing.T) {
t.Fatal(err)
}

img, err := bundle.BuildTektonBundle([]string{examplePullTask, examplePullPipeline}, &bytes.Buffer{})
img, err := bundle.BuildTektonBundle([]string{examplePullTask, examplePullPipeline}, nil, &bytes.Buffer{})
if err != nil {
t.Fatal(err)
}
Expand Down
12 changes: 9 additions & 3 deletions pkg/cmd/bundle/push.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/spf13/cobra"
"github.com/tektoncd/cli/pkg/bundle"
"github.com/tektoncd/cli/pkg/cli"
"github.com/tektoncd/cli/pkg/params"
)

type pushOptions struct {
Expand All @@ -31,6 +32,8 @@ type pushOptions struct {
bundleContents []string
bundleContentPaths []string
remoteOptions bundle.RemoteOptions
annotationParams []string
annotations map[string]string
}

func pushCommand(_ cli.Params) *cobra.Command {
Expand Down Expand Up @@ -80,14 +83,15 @@ Input:
},
}
c.Flags().StringSliceVarP(&opts.bundleContentPaths, "filenames", "f", []string{}, "List of fully-qualified file paths containing YAML or JSON defined Tekton objects to include in this bundle")
c.Flags().StringSliceVarP(&opts.annotationParams, "annotate", "", []string{}, "OCI Manifest annotation in the form of key=value to be added to the OCI image. Can be provided multiple times to add multiple annotations.")
bundle.AddRemoteFlags(c.Flags(), &opts.remoteOptions)

return c
}

// Reads the positional arguments and the `-f` flag to fill in the `bunldeContents` parameter with all of the raw Tekton
// contents.
func (p *pushOptions) parseArgsAndFlags(args []string) error {
func (p *pushOptions) parseArgsAndFlags(args []string) (err error) {
p.ref, _ = name.ParseReference(args[0], name.StrictValidation, name.Insecure)

// If there are file paths specified, then read them and include their contents.
Expand All @@ -109,7 +113,9 @@ func (p *pushOptions) parseArgsAndFlags(args []string) error {
p.bundleContents = append(p.bundleContents, string(contents))
}

return nil
p.annotations, err = params.ParseParams(p.annotationParams)

return err
}

// Run performs the principal logic of reading and parsing the input, creating the bundle, and publishing it.
Expand All @@ -118,7 +124,7 @@ func (p *pushOptions) Run(args []string) error {
return err
}

img, err := bundle.BuildTektonBundle(p.bundleContents, p.stream.Out)
img, err := bundle.BuildTektonBundle(p.bundleContents, p.annotations, p.stream.Out)
if err != nil {
return err
}
Expand Down
27 changes: 23 additions & 4 deletions pkg/cmd/bundle/push_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,12 @@ var (

func TestPushCommand(t *testing.T) {
testcases := []struct {
name string
files map[string]string
stdin string
expectedContents map[string]expected
name string
files map[string]string
stdin string
annotations []string
expectedContents map[string]expected
expectedAnnotations map[string]string
}{
{
name: "single-input",
Expand All @@ -71,6 +73,18 @@ func TestPushCommand(t *testing.T) {
stdin: exampleTask,
expectedContents: map[string]expected{exampleTaskExpected.name: exampleTaskExpected},
},
{
name: "with-annotations",
files: map[string]string{
"simple.yaml": exampleTask,
},
annotations: []string{"org.opencontainers.image.license=Apache-2.0", "org.opencontainers.image.url = https://example.org"},
expectedContents: map[string]expected{exampleTaskExpected.name: exampleTaskExpected},
expectedAnnotations: map[string]string{
"org.opencontainers.image.license": "Apache-2.0",
"org.opencontainers.image.url": "https://example.org",
},
},
}

for _, tc := range testcases {
Expand Down Expand Up @@ -112,6 +126,7 @@ func TestPushCommand(t *testing.T) {
Err: &bytes.Buffer{},
},
bundleContentPaths: paths,
annotationParams: tc.annotations,
remoteOptions: bundle.RemoteOptions{},
}
if err := opts.Run([]string{ref}); err != nil {
Expand All @@ -138,6 +153,10 @@ func TestPushCommand(t *testing.T) {
t.Errorf("Expected %d layers but found %d", len(tc.expectedContents), len(manifest.Layers))
}

if len(manifest.Annotations) != len(tc.expectedAnnotations) || fmt.Sprint(manifest.Annotations) != fmt.Sprint(tc.expectedAnnotations) {
t.Errorf("Requested annotations were not set wanted: %s, got %s", tc.expectedAnnotations, manifest.Annotations)
}

for i, l := range manifest.Layers {
title, ok := l.Annotations[tkremote.TitleAnnotation]
if !ok {
Expand Down
7 changes: 6 additions & 1 deletion pkg/params/mergeparams.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,12 @@ func ParseParams(params []string) (map[string]string, error) {
if len(r) != 2 {
return nil, errors.New(invalidParam + p)
}
parsedParams[r[0]] = r[1]
key := strings.TrimSpace(r[0])
val := strings.TrimSpace(r[1])
if key == "" {
return nil, errors.New(invalidParam + p)
}
vinamra28 marked this conversation as resolved.
Show resolved Hide resolved
parsedParams[key] = val
}
return parsedParams, nil
}
54 changes: 45 additions & 9 deletions pkg/params/mergeparams_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,15 +246,51 @@ func Test_parseParam(t *testing.T) {
}

func Test_ParseParams(t *testing.T) {
t.Run("happy day", func(t *testing.T) {
pass := []string{"abc=bcd", "one=two"}
got, err := ParseParams(pass)
if err != nil {
t.Errorf("unexpected error: %v", err)
}
if len(got) != 2 {
t.Errorf("expected two elements, got: %v", got)
}
test.AssertOutput(t, "bcd", got["abc"])
test.AssertOutput(t, "two", got["one"])
})

pass := []string{"abc=bcd", "one=two"}
got, _ := ParseParams(pass)
test.AssertOutput(t, "bcd", got["abc"])
t.Run("missing =", func(t *testing.T) {
pass := []string{"abc"}
_, err := ParseParams(pass)
if err == nil {
t.Errorf("Expected error")
}
test.AssertOutput(t, "invalid input format for param parameter: abc", err.Error())
})

pass = []string{"abc"}
_, err := ParseParams(pass)
if err == nil {
t.Errorf("Expected error")
}
test.AssertOutput(t, "invalid input format for param parameter: abc", err.Error())
t.Run("missing key and value", func(t *testing.T) {
pass := []string{"="}
_, err := ParseParams(pass)
if err == nil {
t.Errorf("Expected error")
}
test.AssertOutput(t, "invalid input format for param parameter: =", err.Error())
})

t.Run("missing key", func(t *testing.T) {
pass := []string{"=val"}
_, err := ParseParams(pass)
if err == nil {
t.Errorf("Expected error")
}
test.AssertOutput(t, "invalid input format for param parameter: =val", err.Error())
})

t.Run("empty value", func(t *testing.T) {
got, err := ParseParams([]string{"key="})
if err != nil {
t.Errorf("unexpected error: %v", err)
}
test.AssertOutput(t, "", got["key"])
})
}
Loading