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 v1 for sign and verify command #2216

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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_pipeline_sign.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ or using kms

```
--allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
--api-version string apiVersion of the Pipeline to be signed (default "v1")
-f, --file-name string Fle name of the signed pipeline, using the original file name will overwrite the file
-h, --help help for sign
-K, --key-file string Key file
Expand Down
1 change: 1 addition & 0 deletions docs/cmd/tkn_pipeline_verify.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ or using kms

```
--allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
--api-version string apiVersion of the Pipeline to be verified (default "v1")
-h, --help help for verify
-K, --key-file string Key file
-m, --kms-key string KMS key url
Expand Down
1 change: 1 addition & 0 deletions docs/cmd/tkn_task_sign.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ or using kms

```
--allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
--api-version string apiVersion of the Task to be signed (default "v1")
-f, --file-name string file name of the signed task, using the original file name will overwrite the file
-h, --help help for sign
-K, --key-file string Key file
Expand Down
1 change: 1 addition & 0 deletions docs/cmd/tkn_task_verify.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ or using kms

```
--allow-missing-template-keys If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats. (default true)
--api-version string apiVersion of the Task to be verified (default "v1")
-h, --help help for verify
-K, --key-file string Key file
-m, --kms-key string KMS key url
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/tkn-pipeline-sign.1
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ For KMS:
\fB\-\-allow\-missing\-template\-keys\fP[=true]
If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats.

.PP
\fB\-\-api\-version\fP="v1"
apiVersion of the Pipeline to be signed

.PP
\fB\-f\fP, \fB\-\-file\-name\fP=""
Fle name of the signed pipeline, using the original file name will overwrite the file
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/tkn-pipeline-verify.1
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ For KMS:
\fB\-\-allow\-missing\-template\-keys\fP[=true]
If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats.

.PP
\fB\-\-api\-version\fP="v1"
apiVersion of the Pipeline to be verified

.PP
\fB\-h\fP, \fB\-\-help\fP[=false]
help for verify
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/tkn-task-sign.1
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ For KMS:
\fB\-\-allow\-missing\-template\-keys\fP[=true]
If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats.

.PP
\fB\-\-api\-version\fP="v1"
apiVersion of the Task to be signed

.PP
\fB\-f\fP, \fB\-\-file\-name\fP=""
file name of the signed task, using the original file name will overwrite the file
Expand Down
4 changes: 4 additions & 0 deletions docs/man/man1/tkn-task-verify.1
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ For KMS:
\fB\-\-allow\-missing\-template\-keys\fP[=true]
If true, ignore any errors in templates when a field or map key is missing in the template. Only applies to golang and jsonpath output formats.

.PP
\fB\-\-api\-version\fP="v1"
apiVersion of the Task to be verified

.PP
\fB\-h\fP, \fB\-\-help\fP[=false]
help for verify
Expand Down
13 changes: 11 additions & 2 deletions pkg/cmd/pipeline/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"github.com/spf13/cobra"
"github.com/tektoncd/cli/pkg/cli"
"github.com/tektoncd/cli/pkg/trustedresources"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
cliopts "k8s.io/cli-runtime/pkg/genericclioptions"
"sigs.k8s.io/yaml"
)
Expand All @@ -31,6 +33,7 @@ type signOptions struct {
keyfile string
kmsKey string
targetFile string
apiVersion string
}

func signCommand() *cobra.Command {
Expand Down Expand Up @@ -70,7 +73,13 @@ or using kms
return err
}

crd := &v1beta1.Pipeline{}
var crd metav1.Object
Copy link
Contributor

@piyush-garg piyush-garg Jan 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are reading the file provided by user as input, we can get the info about version from file content. Correct me if this is wrong. Why we need to take the input from user?

Also --pipeline-version flag does not look make sense for task commands

Copy link
Member

@chmouel chmouel Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming is hard 😅 I think the idea is to convey which tektoncd/pipeline version we want

but yeah that may be confusing?

any other suggestions? --crd-version perhaps ?

actually forget what i said, you are acutally right we can just autodetect this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion, but I don't know what would be the best way to do this.
How to read the version info from the file content? Scan the string? How to get the version v1 for this case?
e.g.

# apiVersion: v1beta1
apiVersion: v1
...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--pipeline-version means (Tekton) Pipeline Version, it was suggested in the previous comment of this PR. Maybe rename it to --api-version?Wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replied here #2216 (comment)

It does omit the commented one than in file

if opts.apiVersion == "v1beta1" {
crd = &v1beta1.Pipeline{}
} else {
crd = &v1.Pipeline{}
}

if err := yaml.Unmarshal(b, &crd); err != nil {
return fmt.Errorf("error unmarshalling Pipeline: %v", err)
}
Expand All @@ -87,7 +96,7 @@ or using kms
c.Flags().StringVarP(&opts.keyfile, "key-file", "K", "", "Key file")
c.Flags().StringVarP(&opts.kmsKey, "kms-key", "m", "", "KMS key url")
c.Flags().StringVarP(&opts.targetFile, "file-name", "f", "", "Fle name of the signed pipeline, using the original file name will overwrite the file")

c.Flags().StringVarP(&opts.apiVersion, "api-version", "", "v1", "apiVersion of the Pipeline to be signed")
return c
}

Expand Down
70 changes: 43 additions & 27 deletions pkg/cmd/pipeline/sign_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package pipeline

import (
"context"
"fmt"
"os"
"path/filepath"
"testing"
Expand All @@ -28,37 +29,52 @@ import (
func TestSign(t *testing.T) {
ctx := context.Background()
p := &test.Params{}

task := Command(p)

pipeline := Command(p)
os.Setenv("PRIVATE_PASSWORD", "1234")
tmpDir := t.TempDir()
targetFile := filepath.Join(tmpDir, "signed.yaml")
out, err := test.ExecuteCommand(task, "sign", "testdata/pipeline.yaml", "-K", "testdata/cosign.key", "-f", targetFile)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := "*Warning*: This is an experimental command, it's usage and behavior can change in the next release(s)\nPipeline testdata/pipeline.yaml is signed successfully \n"
test.AssertOutput(t, expected, out)

// verify the signed task
verifier, err := cosignsignature.LoadPublicKey(ctx, "testdata/cosign.pub")
if err != nil {
t.Errorf("error getting verifier from key file: %v", err)
}
testcases := []struct {
name string
taskFile string
apiVersion string
}{{
name: "sign and verify v1beta1 Pipeline",
taskFile: "testdata/pipeline.yaml",
apiVersion: "v1beta1",
}, {
name: "sign and verify v1 Pipeline",
taskFile: "testdata/pipeline-v1.yaml",
apiVersion: "v1",
}}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
tmpDir := t.TempDir()
targetFile := filepath.Join(tmpDir, "signed.yaml")
out, err := test.ExecuteCommand(pipeline, "sign", tc.taskFile, "-K", "testdata/cosign.key", "-f", targetFile, "--api-version", tc.apiVersion)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := fmt.Sprintf("*Warning*: This is an experimental command, it's usage and behavior can change in the next release(s)\nPipeline %s is signed successfully \n", tc.taskFile)
test.AssertOutput(t, expected, out)

signed, err := os.ReadFile(targetFile)
if err != nil {
t.Fatalf("error reading file: %v", err)
}
// verify the signed task
verifier, err := cosignsignature.LoadPublicKey(ctx, "testdata/cosign.pub")
if err != nil {
t.Errorf("error getting verifier from key file: %v", err)
}

target, signature, err := trustedresources.UnmarshalCRD(signed, "Pipeline")
if err != nil {
t.Fatalf("error unmarshalling crd: %v", err)
}
signed, err := os.ReadFile(targetFile)
if err != nil {
t.Fatalf("error reading file: %v", err)
}

if err := trustedresources.VerifyInterface(target, verifier, signature); err != nil {
t.Fatalf("VerifyInterface get error: %v", err)
}
target, signature, err := trustedresources.UnmarshalCRD(signed, "Pipeline", tc.apiVersion)
if err != nil {
t.Fatalf("error unmarshalling crd: %v", err)
}

if err := trustedresources.VerifyInterface(target, verifier, signature); err != nil {
t.Fatalf("VerifyInterface get error: %v", err)
}
})
}
}
25 changes: 25 additions & 0 deletions pkg/cmd/pipeline/testdata/signed-v1.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
apiVersion: tekton.dev/v1
kind: Pipeline
metadata:
annotations:
tekton.dev/signature: MEUCIQD3tcptnk2F+9ru5gNUi91K2NPe59Dk28lwaHEQzScnOQIgL+KpDuGBf67FHGrh34cZRHVmPuYzOzPUbmvealAJPvE=
creationTimestamp: null
name: test-pipeline
spec:
tasks:
- name: build-skaffold-web
params:
- name: pathToDockerFile
value: Dockerfile
- name: pathToContext
value: /workspace/docker-source/examples/microservices/leeroy-web
taskRef:
name: build-docker-image-from-git-source
- name: deploy-web
params:
- name: path
value: /workspace/source/examples/microservices/leeroy-web/kubernetes/deployment.yaml
- name: yamlPathToImage
value: spec.template.spec.containers[0].image
taskRef:
name: deploy-using-kubectl
15 changes: 12 additions & 3 deletions pkg/cmd/pipeline/verify.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,17 @@ import (
"github.com/spf13/cobra"
"github.com/tektoncd/cli/pkg/cli"
"github.com/tektoncd/cli/pkg/trustedresources"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
cliopts "k8s.io/cli-runtime/pkg/genericclioptions"
"sigs.k8s.io/yaml"
)

type verifyOptions struct {
keyfile string
kmsKey string
keyfile string
kmsKey string
apiVersion string
}

func verifyCommand() *cobra.Command {
Expand Down Expand Up @@ -68,7 +71,12 @@ or using kms
return err
}

crd := &v1beta1.Pipeline{}
var crd metav1.Object
if opts.apiVersion == "v1beta1" {
crd = &v1beta1.Pipeline{}
} else {
crd = &v1.Pipeline{}
}
if err := yaml.Unmarshal(b, &crd); err != nil {
log.Fatalf("error unmarshalling Pipeline: %v", err)
return err
Expand All @@ -85,5 +93,6 @@ or using kms
f.AddFlags(c)
c.Flags().StringVarP(&opts.keyfile, "key-file", "K", "", "Key file")
c.Flags().StringVarP(&opts.kmsKey, "kms-key", "m", "", "KMS key url")
c.Flags().StringVarP(&opts.apiVersion, "api-version", "", "v1", "apiVersion of the Pipeline to be verified")
return c
}
30 changes: 23 additions & 7 deletions pkg/cmd/pipeline/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package pipeline

import (
"fmt"
"os"
"testing"

Expand All @@ -23,15 +24,30 @@ import (

func TestVerify(t *testing.T) {
p := &test.Params{}

pipeline := Command(p)

os.Setenv("PRIVATE_PASSWORD", "1234")

out, err := test.ExecuteCommand(pipeline, "verify", "testdata/signed.yaml", "-K", "testdata/cosign.pub")
if err != nil {
t.Errorf("Unexpected error: %v", err)
testcases := []struct {
name string
taskFile string
apiVersion string
}{{
name: "verify v1beta1 Pipeline",
taskFile: "testdata/signed.yaml",
apiVersion: "v1beta1",
}, {
name: "verify v1 Pipeline",
taskFile: "testdata/signed-v1.yaml",
apiVersion: "v1",
}}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
out, err := test.ExecuteCommand(pipeline, "verify", tc.taskFile, "-K", "testdata/cosign.pub", "--api-version", tc.apiVersion)
if err != nil {
t.Errorf("Unexpected error: %v", err)
}
expected := fmt.Sprintf("*Warning*: This is an experimental command, it's usage and behavior can change in the next release(s)\nPipeline %s passes verification \n", tc.taskFile)
test.AssertOutput(t, expected, out)
})
}
expected := "*Warning*: This is an experimental command, it's usage and behavior can change in the next release(s)\nPipeline testdata/signed.yaml passes verification \n"
test.AssertOutput(t, expected, out)
}
12 changes: 10 additions & 2 deletions pkg/cmd/task/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import (
"github.com/spf13/cobra"
"github.com/tektoncd/cli/pkg/cli"
"github.com/tektoncd/cli/pkg/trustedresources"
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
cliopts "k8s.io/cli-runtime/pkg/genericclioptions"
"sigs.k8s.io/yaml"
)
Expand All @@ -36,6 +38,7 @@ type signOptions struct {
keyfile string
kmsKey string
targetFile string
apiVersion string
}

func signCommand() *cobra.Command {
Expand Down Expand Up @@ -74,8 +77,13 @@ or using kms
log.Fatalf("error reading file: %v", err)
return err
}
var crd metav1.Object
if opts.apiVersion == "v1beta1" {
crd = &v1beta1.Task{}
} else {
crd = &v1.Task{}
}

crd := &v1beta1.Task{}
if err := yaml.Unmarshal(b, &crd); err != nil {
return fmt.Errorf("error unmarshalling Task: %v", err)
}
Expand All @@ -91,6 +99,6 @@ or using kms
c.Flags().StringVarP(&opts.keyfile, "key-file", "K", "", "Key file")
c.Flags().StringVarP(&opts.kmsKey, "kms-key", "m", "", "KMS key url")
c.Flags().StringVarP(&opts.targetFile, "file-name", "f", "", "file name of the signed task, using the original file name will overwrite the file")

c.Flags().StringVarP(&opts.apiVersion, "api-version", "", "v1", "apiVersion of the Task to be signed")
return c
}
Loading