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

fix: reference correct secret parts in errors #198

Merged
merged 3 commits into from
Sep 8, 2021
Merged

Conversation

wass3r
Copy link
Collaborator

@wass3r wass3r commented Sep 4, 2021

fixes go-vela/community#387

When we call .ParseOrg, we pass in the current repo's information (eg. https://github.com/go-vela/pkg-executor/blob/57b690402cf77554746eebe9e870ae7a3ec1e37d/executor/linux/secret.go#L196 - passing in current repo's org).

When a user provides a secret from a different org via the key property, the error prints the current org the user is trying to use the secret from instead of the org that they are referencing in the key.

for example:

Assuming the following pipeline is used in github.com/go-vela/test:

version: "1"

steps:
  - name: test
    image: alpine:latest
    pull: true
    command:
      - echo hi

secrets:
  - name: org-secret
    key: wass3r/secret
    type: org
    engine: native

The error would print:

error:invalid organization in key: go-vela

instead of:

error:invalid organization in key: wass3r

@wass3r wass3r requested a review from a team as a code owner September 4, 2021 08:16
@codecov
Copy link

codecov bot commented Sep 4, 2021

Codecov Report

Merging #198 (333e09e) into master (e71f8bc) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #198   +/-   ##
=======================================
  Coverage   96.97%   96.97%           
=======================================
  Files          53       53           
  Lines        5462     5462           
=======================================
  Hits         5297     5297           
  Misses        119      119           
  Partials       46       46           
Impacted Files Coverage Δ
pipeline/secret.go 90.10% <100.00%> (ø)

@wass3r
Copy link
Collaborator Author

wass3r commented Sep 5, 2021

one thought.. should we print the whole supplied key value instead?

Copy link
Contributor

@jbrockopp jbrockopp left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@matt-fevold matt-fevold left a comment

Choose a reason for hiding this comment

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

LGTM

@matt-fevold
Copy link

one thought.. should we print the whole supplied key value instead?

Not sure 🤔 I think what you've got already should help reduce confusion a lot.

@wass3r wass3r merged commit c8edeac into master Sep 8, 2021
@wass3r wass3r deleted the fix/error-msgs branch September 8, 2021 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

invalid organization in key prints current org
3 participants