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: --target-name bug #3618

Merged
merged 1 commit into from
Aug 25, 2022
Merged

fix: --target-name bug #3618

merged 1 commit into from
Aug 25, 2022

Conversation

YairZ101
Copy link
Contributor

@YairZ101 YairZ101 commented Aug 23, 2022

What does this PR do?

IaC has recently introduced a new flag in the CLI called --target-name that as the name suggest allows the user to overwrite the target name in the platform.
We currently have a bug where when scanning a directory that is a root of a repo the target name is not getting overridden with the one that the user set by using the flag.
This PR adds the ability to add the name field to the target together with git information, in registry we can use the name to overwrite the target name.

How should this be manually tested?

  1. Run registry using the fix PR linked below
  2. cd to a directory that is the root of a repo
  3. Run snyk-dev iac test --report --target-name=test
  4. Open the UI and check for the correct target name

Any background context you want to provide?

The fix in this PR is dependent on the fix in https://github.com/snyk/registry/pull/28707.

What are the relevant tickets?

https://snyksec.atlassian.net/jira/software/c/projects/CFG/boards/301?modal=detail&selectedIssue=CFG-2101

@@ -20,7 +20,7 @@ export interface ContainerTarget {
image: string;
}

export interface UnknownTarget {
export interface NamedTarget extends GitTarget {
Copy link
Contributor Author

@YairZ101 YairZ101 Aug 24, 2022

Choose a reason for hiding this comment

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

I'm not 100% sure if this is the best approach here, maybe adding name as an optional field to GitTarget is a better option?

@YairZ101 YairZ101 force-pushed the fix/target-name branch 2 times, most recently from 127e9f0 to 090d2dd Compare August 24, 2022 10:10
@YairZ101 YairZ101 marked this pull request as ready for review August 24, 2022 10:18
@YairZ101 YairZ101 requested review from a team as code owners August 24, 2022 10:18
@YairZ101 YairZ101 requested review from karniwl, robcresswell, Jdunsby, robh-snyk, ofekatr and francescomari and removed request for karniwl and ofekatr August 24, 2022 10:18
Copy link
Contributor

@orsagie orsagie left a comment

Choose a reason for hiding this comment

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

LGTM

@YairZ101 YairZ101 merged commit e1132e1 into master Aug 25, 2022
@YairZ101 YairZ101 deleted the fix/target-name branch August 25, 2022 12:44
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.

3 participants