Skip to content

Commit

Permalink
add lint warning when secret used in docker-push vars
Browse files Browse the repository at this point in the history
  • Loading branch information
@robwhitby
robwhitby committed Sep 8, 2023
1 parent 4b23214 commit c3da147
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 36 deletions.
8 changes: 7 additions & 1 deletion linters/docker-push.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import (
"github.com/springernature/halfpipe/manifest"
)

func LintDockerPushTask(docker manifest.DockerPush, manifest manifest.Manifest, fs afero.Afero) (errs []error) {
func LintDockerPushTask(docker manifest.DockerPush, fs afero.Afero) (errs []error) {
if docker.Image == "" {
errs = append(errs, NewErrMissingField("image"))
} else {
Expand Down Expand Up @@ -68,5 +68,11 @@ func LintDockerPushTask(docker manifest.DockerPush, manifest manifest.Manifest,
}
}

for k, v := range docker.Vars {
if strings.HasPrefix(v, "((") && strings.HasSuffix(v, "))") && !strings.HasPrefix(k, "ARTIFACTORY_") {
errs = append(errs, ErrDockerVarSecret.WithValue(k).AsWarning())
}
}

return errs
}
68 changes: 42 additions & 26 deletions linters/docker-push_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ var emptyManifest = manifest.Manifest{}
func TestDockerPushTaskWithEmptyTask(t *testing.T) {
fs := afero.Afero{Fs: afero.NewMemMapFs()}

errors := LintDockerPushTask(manifest.DockerPush{}, emptyManifest, fs)
errors := LintDockerPushTask(manifest.DockerPush{}, fs)
assertContainsError(t, errors, NewErrMissingField("image"))
}

Expand All @@ -25,7 +25,7 @@ func TestDockerPushTaskWithBadRepo(t *testing.T) {
Image: "asd",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assertContainsError(t, errors, ErrInvalidField.WithValue("image"))
}

Expand All @@ -40,7 +40,7 @@ func TestDockerPushTaskWithoutTeamDirectoryInHalfpipeRepo(t *testing.T) {
DockerfilePath: "Dockerfile",
}

errs := LintDockerPushTask(task, emptyManifest, fs)
errs := LintDockerPushTask(task, fs)
assertContainsError(t, errs, ErrInvalidField.WithValue("image"))
}

Expand All @@ -55,7 +55,7 @@ func TestDockerPushTaskWithTeamDirectoryInHalfpipeRepo(t *testing.T) {
DockerfilePath: "Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
}

Expand All @@ -70,7 +70,7 @@ func TestDockerPushTaskWithoutTeamDirectoryInGCRRepo(t *testing.T) {
DockerfilePath: "Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
}

Expand All @@ -85,7 +85,7 @@ func TestDockerPushTaskWhenDockerfileIsMissing(t *testing.T) {
DockerfilePath: "Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assertContainsError(t, errors, ErrFileNotFound)
})

Expand All @@ -100,7 +100,7 @@ func TestDockerPushTaskWhenDockerfileIsMissing(t *testing.T) {
RestoreArtifacts: true,
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assertNotContainsError(t, errors, ErrFileNotFound)
})
}
Expand All @@ -122,7 +122,7 @@ func TestDockerPushTaskWithCorrectData(t *testing.T) {
DockerfilePath: "Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
})

Expand All @@ -141,7 +141,7 @@ func TestDockerPushTaskWithCorrectData(t *testing.T) {
DockerfilePath: "dockerfile/Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
})

Expand All @@ -160,7 +160,7 @@ func TestDockerPushTaskWithCorrectData(t *testing.T) {
DockerfilePath: "../dockerfile/Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
})

Expand All @@ -182,7 +182,7 @@ func TestDockerPushWithBuildPath(t *testing.T) {
BuildPath: "buildPathDoesntExist",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 1)
assertContainsError(t, errors, ErrInvalidField.WithValue("build_path"))
})
Expand All @@ -206,7 +206,7 @@ func TestDockerPushWithBuildPath(t *testing.T) {
BuildPath: buildPath,
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 1)
assertContainsError(t, errors, ErrInvalidField.WithValue("build_path"))
})
Expand All @@ -230,7 +230,7 @@ func TestDockerPushWithBuildPath(t *testing.T) {
BuildPath: buildPath,
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
})

Expand All @@ -253,7 +253,7 @@ func TestDockerPushWithBuildPath(t *testing.T) {
BuildPath: buildPath,
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
})

Expand All @@ -269,7 +269,7 @@ func TestDockerPushWithBuildPath(t *testing.T) {
RestoreArtifacts: true,
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Empty(t, errors)
})

Expand All @@ -291,15 +291,15 @@ func TestDockerPushRetries(t *testing.T) {
}

task.Retries = -1
errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assertContainsError(t, errors, ErrInvalidField.WithValue("retries"))

task.Retries = 6
errors = LintDockerPushTask(task, emptyManifest, fs)
errors = LintDockerPushTask(task, fs)
assertContainsError(t, errors, ErrInvalidField.WithValue("retries"))

task.Retries = 4
errors = LintDockerPushTask(task, emptyManifest, fs)
errors = LintDockerPushTask(task, fs)
assert.Len(t, errors, 0)
}

Expand All @@ -315,7 +315,7 @@ func TestDockerPushTag(t *testing.T) {
DockerfilePath: "Dockerfile",
}

errors := LintDockerPushTask(task, emptyManifest, fs)
errors := LintDockerPushTask(task, fs)
assert.Empty(t, errors)
})

Expand All @@ -331,7 +331,7 @@ func TestDockerPushTag(t *testing.T) {
Tag: "yolo",
}

errs := LintDockerPushTask(task, emptyManifest, fs)
errs := LintDockerPushTask(task, fs)
assertContainsError(t, errs, ErrDockerPushTag)
})
}
Expand All @@ -349,9 +349,7 @@ func TestMultiplePlatforms(t *testing.T) {
Platforms: []string{"linux/arm64", "linux/amd64"},
}

m := manifest.Manifest{Platform: "actions"}

errors := LintDockerPushTask(task, m, fs)
errors := LintDockerPushTask(task, fs)
assert.Empty(t, errors)
})

Expand All @@ -367,9 +365,27 @@ func TestMultiplePlatforms(t *testing.T) {
Platforms: []string{"linux/ad64"},
}

m := manifest.Manifest{Platform: "actions"}

errors := LintDockerPushTask(task, m, fs)
errors := LintDockerPushTask(task, fs)
assertContainsError(t, errors, ErrDockerPlatformUnknown)
})
}

func TestSecrets(t *testing.T) {
t.Run("no secrets in vars please", func(t *testing.T) {
fs := afero.Afero{Fs: afero.NewMemMapFs()}
fs.WriteFile("Dockerfile", []byte("FROM ubuntu"), 0777)

task := manifest.DockerPush{
Image: "asd/asd",
Username: "asd",
Password: "asdf",
DockerfilePath: "Dockerfile",
Platforms: []string{"linux/arm64", "linux/amd64"},
Vars: manifest.Vars{"var1": "((a.secret))"},
}

errors := LintDockerPushTask(task, fs)
assertContainsError(t, errors, ErrDockerVarSecret.WithValue("var1").AsWarning())
})

}
7 changes: 4 additions & 3 deletions linters/errors.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ var (

ErrDockerPlatformUnknown = newError("only linux/amd64 and/or linux/arm64 are supported")
ErrDockerComposeVersion = newError("the docker-compose file version used is deprecated. All services must be under the 'services' key and 'Version' must be '2' or higher. Please see <https://docs.docker.com/compose/compose-file/compose-versioning/#versioning>")
ErrMultipleTriggers = newError("cannot have multiple triggers of this type")
ErrDockerVarSecret = newError("docker build var uses secret")

ErrMultipleTriggers = newError("cannot have multiple triggers of this type")

ErrVelaVariableMissing = newError("vela manifest variable is not specified in halfpipe manifest")
ErrVelaNamespace = newError("vela namespace must start with 'katee-'")
Expand Down Expand Up @@ -75,8 +77,7 @@ func (e Error) Error() string {
}

func (e Error) AsWarning() Error {
e.level = "warning"
return e
return Error{err: e.err, level: "warning", value: e.value}
}

func (e Error) IsWarning() bool {
Expand Down
4 changes: 2 additions & 2 deletions linters/task.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ type taskLinter struct {
lintDeployCFTask func(task manifest.DeployCF, readCfManifest cf.ManifestReader, fs afero.Afero) []error
lintDeployKateeTask func(task manifest.DeployKatee, man manifest.Manifest, fs afero.Afero) []error
LintPrePromoteTask func(task manifest.Task) []error
lintDockerPushTask func(task manifest.DockerPush, man manifest.Manifest, fs afero.Afero) []error
lintDockerPushTask func(task manifest.DockerPush, fs afero.Afero) []error
lintDockerComposeTask func(task manifest.DockerCompose, fs afero.Afero) []error
lintConsumerIntegrationTestTask func(task manifest.ConsumerIntegrationTest, providerHostRequired bool) []error
lintDeployMLZipTask func(task manifest.DeployMLZip) []error
Expand Down Expand Up @@ -97,7 +97,7 @@ func (linter taskLinter) lintTasks(listName string, ts []manifest.Task, man mani
case manifest.DeployKatee:
errs = linter.lintDeployKateeTask(task, man, linter.Fs)
case manifest.DockerPush:
errs = linter.lintDockerPushTask(task, man, linter.Fs)
errs = linter.lintDockerPushTask(task, linter.Fs)
case manifest.DockerCompose:
errs = linter.lintDockerComposeTask(task, linter.Fs)
case manifest.ConsumerIntegrationTest:
Expand Down
8 changes: 4 additions & 4 deletions linters/task_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func TestCallsOutToTheLintersCorrectly(t *testing.T) {
calledLintPrePromoteTasksNum++
return
},
lintDockerPushTask: func(task manifest.DockerPush, man manifest.Manifest, fs afero.Afero) (errs []error) {
lintDockerPushTask: func(task manifest.DockerPush, fs afero.Afero) (errs []error) {
calledLintDockerPushTask = true
calledLintDockerPushTaskNum++
return
Expand Down Expand Up @@ -232,7 +232,7 @@ func TestMergesTheErrorsAndWarningsCorrectlyWithPrePromote(t *testing.T) {
LintPrePromoteTask: func(tasks manifest.Task) (errs []error) {
return []error{prePromoteErr, prePromoteWarn}
},
lintDockerPushTask: func(task manifest.DockerPush, man manifest.Manifest, fs afero.Afero) (errs []error) {
lintDockerPushTask: func(task manifest.DockerPush, fs afero.Afero) (errs []error) {
return []error{dockerPushErr, dockerPushWarn}
},
lintDeployMLZipTask: func(task manifest.DeployMLZip) (errs []error) {
Expand Down Expand Up @@ -323,7 +323,7 @@ func TestMergesTheErrorsAndWarningsCorrectlyWithParallel(t *testing.T) {
LintPrePromoteTask: func(tasks manifest.Task) (errs []error) {
return []error{prePromoteErr, prePromoteWarn}
},
lintDockerPushTask: func(task manifest.DockerPush, man manifest.Manifest, fs afero.Afero) (errs []error) {
lintDockerPushTask: func(task manifest.DockerPush, fs afero.Afero) (errs []error) {
return []error{dockerPushErr, dockerPushWarn}
},
lintDeployMLZipTask: func(task manifest.DeployMLZip) (errs []error) {
Expand Down Expand Up @@ -557,7 +557,7 @@ func TestLintTimeout(t *testing.T) {
return
},
LintPrePromoteTask: func(task manifest.Task) (errs []error) { return },
lintDockerPushTask: func(task manifest.DockerPush, man manifest.Manifest, fs afero.Afero) (errs []error) {
lintDockerPushTask: func(task manifest.DockerPush, fs afero.Afero) (errs []error) {
return
},
lintDockerComposeTask: func(task manifest.DockerCompose, fs afero.Afero) (errs []error) {
Expand Down

0 comments on commit c3da147

Please sign in to comment.