Skip to content

Commit ae2a834

Browse files
arifash01arifash01
committed
Adding v1 ApiVersioning compatibility for catlins pre-commit hooks
1 parent 3f51bad commit ae2a834

3 files changed

Lines changed: 110 additions & 39 deletions

File tree

‎pkg/linter/script.go‎

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"github.com/tektoncd/catlin/pkg/parser"
2424
"github.com/tektoncd/catlin/pkg/validator"
25+
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2526
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
2627
)
2728

@@ -88,20 +89,20 @@ func NewScriptLinter(r *parser.Resource) *taskLinter {
8889
}
8990

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

9495
// use /bin/sh by default if no shbang
95-
if s.Script[0:2] != "#!" {
96-
s.Script = "#!/usr/bin/env sh\n" + s.Script
96+
if script[0:2] != "#!" {
97+
script = "#!/usr/bin/env sh\n" + script
9798
} else { // using a shbang, check if we have /usr/bin/env
98-
if s.Script[0:14] != "#!/usr/bin/env" {
99+
if script[0:14] != "#!/usr/bin/env" {
99100
result.Warn("step: %s is not using #!/usr/bin/env ", taskName)
100101
}
101102
}
102103

103104
for _, config := range configs {
104-
matched, err := regexp.MatchString(`^#!`+config.regexp+`\n`, s.Script)
105+
matched, err := regexp.MatchString(`^#!`+config.regexp+`\n`, script)
105106
if err != nil {
106107
result.Error("Invalid regexp: %s", config.regexp)
107108
return result
@@ -123,7 +124,7 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c
123124
return result
124125
}
125126
defer os.Remove(tmpfile.Name()) // clean up
126-
if _, err := tmpfile.Write([]byte(s.Script)); err != nil {
127+
if _, err := tmpfile.Write([]byte(script)); err != nil {
127128
result.Error("Cannot write to temporary files")
128129
return result
129130
}
@@ -138,7 +139,7 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c
138139
cmd := exec.Command(execpath, append(linter.args, tmpfile.Name())...)
139140
out, err := cmd.CombinedOutput()
140141
if err != nil {
141-
outt := strings.ReplaceAll(string(out), tmpfile.Name(), taskName+"-"+s.Name)
142+
outt := strings.ReplaceAll(string(out), tmpfile.Name(), taskName+"-"+stepName)
142143
result.Error("%s, %s failed:\n%s", execpath, linter.args, outt)
143144
}
144145
}
@@ -148,10 +149,18 @@ func (t *taskLinter) validateScript(taskName string, s v1beta1.Step, configs []c
148149
}
149150

150151
// nolint: staticcheck
151-
func (t *taskLinter) collectOverSteps(steps []v1beta1.Step, name string, result *validator.Result) {
152-
for _, step := range steps {
153-
if step.Script != "" {
154-
result.Append(t.validateScript(name, step, t.configs))
152+
func (t *taskLinter) collectOverSteps(steps interface{}, name string, result *validator.Result) {
153+
if s, ok := steps.([]v1beta1.Step); ok {
154+
for _, step := range s {
155+
if step.Script != "" {
156+
result.Append(t.validateScript(name, step.Script, t.configs, step.Name))
157+
}
158+
}
159+
} else if s, ok := steps.([]v1.Step); ok {
160+
for _, step := range s {
161+
if step.Script != "" {
162+
result.Append(t.validateScript(name, step.Script, t.configs, step.Name))
163+
}
155164
}
156165
}
157166
}
@@ -167,11 +176,17 @@ func (t *taskLinter) Validate() validator.Result {
167176

168177
switch strings.ToLower(t.res.Kind) {
169178
case "task":
170-
task := res.(*v1beta1.Task)
171-
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
179+
if res.(*v1.Task) != nil {
180+
task := res.(*v1.Task)
181+
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
182+
} else {
183+
task := res.(*v1beta1.Task)
184+
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
185+
}
186+
172187
case "clustertask":
173-
task := res.(*v1beta1.ClusterTask)
174-
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
188+
task := res.(*v1beta1.ClusterTask)
189+
t.collectOverSteps(task.Spec.Steps, task.ObjectMeta.Name, &result)
175190
}
176191
return result
177192
}

‎pkg/parser/parser.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,14 @@ import (
2929
"knative.dev/pkg/apis"
3030

3131
"github.com/tektoncd/catlin/pkg/consts"
32+
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
3233
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
3334
)
3435

3536
func registerSchema() {
37+
v1 := runtime.NewSchemeBuilder(v1.AddToScheme)
38+
_ = v1.AddToScheme(scheme.Scheme)
39+
3640
beta1 := runtime.NewSchemeBuilder(v1beta1.AddToScheme)
3741
_ = beta1.AddToScheme(scheme.Scheme)
3842
}
@@ -152,12 +156,19 @@ type tektonResource interface {
152156

153157
// nolint: staticcheck
154158
func typeForKind(kind string) (tektonResource, error) {
159+
isV1 := (&v1.Task{} != nil)
155160
switch kind {
156161
case "Task":
162+
if isV1 {
163+
return &v1.Task{}, nil
164+
}
157165
return &v1beta1.Task{}, nil
158166
case "ClusterTask":
159167
return &v1beta1.ClusterTask{}, nil
160168
case "Pipeline":
169+
if isV1 {
170+
return &v1.Pipeline{}, nil
171+
}
161172
return &v1beta1.Pipeline{}, nil
162173
}
163174

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

167178
func isTektonKind(gvk *schema.GroupVersionKind) bool {
168179
id := gvk.GroupVersion().Identifier()
169-
return id == v1beta1.SchemeGroupVersion.Identifier()
180+
return id == v1.SchemeGroupVersion.Identifier() || id == v1beta1.SchemeGroupVersion.Identifier()
170181
}

‎pkg/validator/task_validator.go‎

Lines changed: 68 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,9 @@ import (
2020
"strings"
2121

2222
"github.com/google/go-containerregistry/pkg/name"
23+
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
2324
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
25+
corev1 "k8s.io/api/core/v1"
2426

2527
"github.com/tektoncd/catlin/pkg/parser"
2628
)
@@ -50,72 +52,115 @@ func (t *taskValidator) Validate() Result {
5052
return result
5153
}
5254

53-
task := res.(*v1beta1.Task)
54-
for _, step := range task.Spec.Steps {
55-
result.Append(t.validateStep(step))
55+
switch task := res.(type) {
56+
case *v1.Task:
57+
for _, step := range task.Spec.Steps {
58+
result.Append(t.validateStep(step))
59+
}
60+
case *v1beta1.Task:
61+
for _, step := range task.Spec.Steps {
62+
result.Append(t.validateStep(step))
63+
}
5664
}
65+
5766
return result
5867
}
5968

60-
func (t *taskValidator) validateStep(s v1beta1.Step) Result {
69+
func (t *taskValidator) validateStep(step interface{}) Result {
6170
result := Result{}
62-
step := s.Name
63-
img := s.Image
71+
72+
var (
73+
version, stepName, img, script string
74+
env []corev1.EnvVar
75+
envFrom []corev1.EnvFromSource
76+
)
77+
78+
switch s := step.(type) {
79+
case v1.Step:
80+
version = "v1"
81+
stepName = s.Name
82+
img = s.Image
83+
env = s.Env
84+
envFrom = s.EnvFrom
85+
script = s.Script
86+
case v1beta1.Step:
87+
version = "v1beta1"
88+
stepName = s.Name
89+
img = s.Image
90+
env = s.Env
91+
envFrom = s.EnvFrom
92+
script = s.Script
93+
default:
94+
return result
95+
}
6496

6597
if _, usesVars := extractExpressionFromString(img, ""); usesVars {
66-
result.Warn("Step %q uses image %q that contains variables; skipping validation", step, img)
98+
result.Warn("Step %q uses image %q that contains variables; skipping validation", stepName, img)
6799
return result
68100
}
69101

70102
if !strings.Contains(img, "/") || !isValidRegistry(img) {
71-
result.Warn("Step %q uses image %q; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0", step, img)
103+
result.Warn("Step %q uses image %q; consider using a fully qualified name - e.g. docker.io/library/ubuntu:1.0", stepName, img)
72104
}
73105

74106
if strings.Contains(img, "@sha256") {
75107
rep, err := name.NewDigest(img, name.WeakValidation)
76108
if err != nil {
77-
result.Error("Step %q uses image %q with an invalid digest. Error: %s", step, img, err)
109+
result.Error("Step %q uses image %q with an invalid digest. Error: %s", stepName, img, err)
78110
return result
79111
}
80112

81113
if !tagWithDigest(rep.String()) {
82-
result.Warn("Step %q uses image %q; consider using a image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde", step, img)
114+
result.Warn("Step %q uses image %q; consider using an image tagged with specific version along with digest eg. abc.io/img:v1@sha256:abcde", stepName, img)
83115
}
84116

85117
return result
86118
}
87119

88120
ref, err := name.NewTag(img, name.WeakValidation)
89121
if err != nil {
90-
result.Error("Step %q uses image %q with an invalid tag. Error: %s", step, img, err)
122+
result.Error("Step %q uses image %q with an invalid tag. Error: %s", stepName, img, err)
91123
return result
92124
}
93125

94126
if strings.EqualFold(ref.Identifier(), "latest") {
95-
result.Error("Step %q uses image %q which must be tagged with a specific version", step, img)
127+
result.Error("Step %q uses image %q which must be tagged with a specific version", stepName, img)
96128
}
97129

98130
// According to [CIS benchmarks](https://cloud.google.com/kubernetes-engine/docs/concepts/cis-benchmarks).
99131
// > 5.4.1 Prefer using secrets as files over secrets as environment variables
100-
for _, env := range s.Env {
101-
if env.ValueFrom == nil || env.ValueFrom.SecretKeyRef == nil {
102-
continue
132+
for _, e := range env {
133+
switch version {
134+
case "v1":
135+
if e.ValueFrom != nil && e.ValueFrom.SecretKeyRef != nil {
136+
result.Warn("Step %q uses secret to populate env %q. Prefer using secrets as files over secrets as environment variables", stepName, e.Name)
137+
}
138+
139+
case "v1beta1":
140+
if e.ValueFrom != nil && e.ValueFrom.SecretKeyRef != nil {
141+
result.Warn("Step %q uses secret to populate env %q. Prefer using secrets as files over secrets as environment variables", stepName, e.Name)
142+
}
103143
}
104-
result.Warn("Step %q uses secret to populate env %q. Prefer using secrets as files over secrets as environment variables", step, env.Name)
105144
}
106-
for _, envFrom := range s.EnvFrom {
107-
if envFrom.SecretRef == nil {
108-
continue
145+
for _, e := range envFrom {
146+
switch version {
147+
case "v1":
148+
if e.SecretRef != nil {
149+
result.Warn("Step %q uses secret as environment variables. Prefer using secrets as files over secrets as environment variables", stepName)
150+
}
151+
case "v1beta1":
152+
if e.SecretRef != nil {
153+
result.Warn("Step %q uses secret as environment variables. Prefer using secrets as files over secrets as environment variables", stepName)
154+
}
109155
}
110-
result.Warn("Step %q uses secret as environment variables. Prefer using secrets as files over secrets as environment variables", step)
111156
}
112157

113-
if s.Script != "" {
114-
expr, _ := extractExpressionFromString(s.Script, "params")
158+
if script != "" {
159+
expr, _ := extractExpressionFromString(script, "params")
115160
if expr != "" {
116161
result.Warn(
117162
"Step %q references %q directly from its script block. For reliability and security, consider putting the param into an environment variable of the Step and accessing that environment variable in your script instead.",
118-
step,
163+
stepName,
119164
expr)
120165
}
121166
}

0 commit comments

Comments
 (0)