Skip to content

Commit

Permalink
Adding v1 ApiVersioning compatibility for catlins pre-commit hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
arifash01 committed Mar 18, 2024
1 parent 3f51bad commit 0b29770
Show file tree
Hide file tree
Showing 5 changed files with 501 additions and 41 deletions.
45 changes: 30 additions & 15 deletions pkg/linter/script.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

"github.com/tektoncd/catlin/pkg/parser"
"github.com/tektoncd/catlin/pkg/validator"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

Expand Down Expand Up @@ -88,20 +89,20 @@ func NewScriptLinter(r *parser.Resource) *taskLinter {
}

// nolint: staticcheck
func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []config) validator.Result {
func (t *taskLinter) validateScript(taskName string, script string, configs []config, stepName string) validator.Result {
result := validator.Result{}

// use /bin/sh by default if no shbang
if s.Script[0:2] != "#!" {
s.Script = "#!/usr/bin/env sh\n" + s.Script
if script[0:2] != "#!" {
script = "#!/usr/bin/env sh\n" + script
} else { // using a shbang, check if we have /usr/bin/env
if s.Script[0:14] != "#!/usr/bin/env" {
if script[0:14] != "#!/usr/bin/env" {
result.Warn("step: %s is not using #!/usr/bin/env ", taskName)
}
}

for _, config := range configs {
matched, err := regexp.MatchString(`^#!`+config.regexp+`\n`, s.Script)
matched, err := regexp.MatchString(`^#!`+config.regexp+`\n`, script)
if err != nil {
result.Error("Invalid regexp: %s", config.regexp)
return result
Expand All @@ -123,7 +124,7 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c
return result
}
defer os.Remove(tmpfile.Name()) // clean up
if _, err := tmpfile.Write([]byte(s.Script)); err != nil {
if _, err := tmpfile.Write([]byte(script)); err != nil {
result.Error("Cannot write to temporary files")
return result
}
Expand All @@ -138,7 +139,7 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c
cmd := exec.Command(execpath, append(linter.args, tmpfile.Name())...)
out, err := cmd.CombinedOutput()
if err != nil {
outt := strings.ReplaceAll(string(out), tmpfile.Name(), taskName+"-"+s.Name)
outt := strings.ReplaceAll(string(out), tmpfile.Name(), taskName+"-"+stepName)
result.Error("%s, %s failed:\n%s", execpath, linter.args, outt)
}
}
Expand All @@ -148,10 +149,18 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c
}

// nolint: staticcheck
func (t *taskLinter) collectOverSteps(steps []v1beta1.Step, name string, result *validator.Result) {
for _, step := range steps {
if step.Script != "" {
result.Append(t.validateScript(name, step, t.configs))
func (t *taskLinter) collectOverSteps(steps interface{}, name string, result *validator.Result) {
if s, ok := steps.([]v1beta1.Step); ok {
for _, step := range s {
if step.Script != "" {
result.Append(t.validateScript(name, step.Script, t.configs, step.Name))
}
}
} else if s, ok := steps.([]v1.Step); ok {
for _, step := range s {
if step.Script != "" {
result.Append(t.validateScript(name, step.Script, t.configs, step.Name))
}
}
}
}
Expand All @@ -167,11 +176,17 @@ func (t *taskLinter) Validate() validator.Result {

switch strings.ToLower(t.res.Kind) {
case "task":
task := res.(*v1beta1.Task)
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
if res.(*v1.Task) != nil {
task := res.(*v1.Task)
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
} else {
task := res.(*v1beta1.Task)
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
}

case "clustertask":
task := res.(*v1beta1.ClusterTask)
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
task := res.(*v1beta1.ClusterTask)
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
}
return result
}
88 changes: 88 additions & 0 deletions pkg/linter/script_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,23 @@ spec:
echo "hello moto"
`

const taskScriptValidatorGoodV1 = `
apiVersion: tekton.dev/v1
kind: Task
metadata:
name: hello-moto
spec:
steps:
- name: s1
image: image1
script: |
#!/usr/bin/env sh
echo "Hello world"
script: |
#!/bin/sh
echo "hello moto"
`

const taskScriptValidatorNoGood = `
apiVersion: tekton.dev/v1beta1
kind: Task
Expand All @@ -58,6 +75,24 @@ spec:
#!/bin/sh
echo "hello world"
`
const taskScriptValidatorNoGoodV1 = `
apiVersion: tekton.dev/v1
kind: Task
metadata:
name: hello-moto
spec:
steps:
- name: nogood
image: image1
script: |
#!/usr/bin/env sh
'
- name: warn
image: image1
script: |
#!/bin/sh
echo "hello world"
`

const clusterTaskTest = `
apiVersion: tekton.dev/v1beta1
Expand All @@ -84,6 +119,17 @@ spec:
taskRef:
name: task-1`

const pipelineWithTaskRefV1 = `
apiVersion: tekton.dev/v1
kind: Pipeline
metadata:
name: pipelien-1
spec:
tasks:
- name: pipeline-1
taskRef:
name: task-1`

var configSh = []config{
{
regexp: `(/usr/bin/env |/bin/)sh`,
Expand All @@ -110,6 +156,20 @@ func TestTaskLint_Script_good(t *testing.T) {
result := tl.Validate()
assert.Equal(t, 0, result.Errors)
}
func TestTaskLint_Script_goodV1(t *testing.T) {
r := strings.NewReader(taskScriptValidatorGoodV1)
parser := parser.ForReader(r)

res, err := parser.Parse()
assert.NilError(t, err)

tl := &taskLinter{
res: res,
configs: configSh,
}
result := tl.Validate()
assert.Equal(t, 0, result.Errors)
}

func TestTaskLint_Script_no_good(t *testing.T) {
r := strings.NewReader(taskScriptValidatorNoGood)
Expand All @@ -125,6 +185,20 @@ func TestTaskLint_Script_no_good(t *testing.T) {
result := tl.Validate()
assert.Equal(t, 1, result.Errors)
}
func TestTaskLint_Script_no_goodV1(t *testing.T) {
r := strings.NewReader(taskScriptValidatorNoGoodV1)
parser := parser.ForReader(r)

res, err := parser.Parse()
assert.NilError(t, err)

tl := &taskLinter{
res: res,
configs: configSh,
}
result := tl.Validate()
assert.Equal(t, 1, result.Errors)
}

func Test_Pipeline_skip(t *testing.T) {
r := strings.NewReader(pipelineWithTaskRef)
Expand All @@ -140,6 +214,20 @@ func Test_Pipeline_skip(t *testing.T) {
result := tl.Validate()
assert.Assert(t, is.Nil(result.Lints))
}
func Test_Pipeline_skipV1(t *testing.T) {
r := strings.NewReader(pipelineWithTaskRefV1)
parser := parser.ForReader(r)

res, err := parser.Parse()
assert.NilError(t, err)

tl := &taskLinter{
res: res,
configs: configSh,
}
result := tl.Validate()
assert.Assert(t, is.Nil(result.Lints))
}

func Test_ClusterTaskParse(t *testing.T) {
r := strings.NewReader(clusterTaskTest)
Expand Down
13 changes: 12 additions & 1 deletion pkg/parser/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,14 @@ import (
"knative.dev/pkg/apis"

"github.com/tektoncd/catlin/pkg/consts"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
)

func registerSchema() {
v1 := runtime.NewSchemeBuilder(v1.AddToScheme)
_ = v1.AddToScheme(scheme.Scheme)

beta1 := runtime.NewSchemeBuilder(v1beta1.AddToScheme)
_ = beta1.AddToScheme(scheme.Scheme)
}
Expand Down Expand Up @@ -152,12 +156,19 @@ type tektonResource interface {

// nolint: staticcheck
func typeForKind(kind string) (tektonResource, error) {
isV1 := (&v1.Task{} != nil)
switch kind {
case "Task":
if isV1 {
return &v1.Task{}, nil
}
return &v1beta1.Task{}, nil
case "ClusterTask":
return &v1beta1.ClusterTask{}, nil
case "Pipeline":
if isV1 {
return &v1.Pipeline{}, nil
}
return &v1beta1.Pipeline{}, nil
}

Expand All @@ -166,5 +177,5 @@ func typeForKind(kind string) (tektonResource, error) {

func isTektonKind(gvk *schema.GroupVersionKind) bool {
id := gvk.GroupVersion().Identifier()
return id == v1beta1.SchemeGroupVersion.Identifier()
return id == v1.SchemeGroupVersion.Identifier() || id == v1beta1.SchemeGroupVersion.Identifier()
}
Loading

0 comments on commit 0b29770

Please sign in to comment.