-
Notifications
You must be signed in to change notification settings - Fork 41
feat: require project name on workflow management #1378
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
Changes from all commits
e7132ea
4bd7321
7740cfa
8c2550d
590d615
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,6 +29,7 @@ func newAttestationInitCmd() *cobra.Command { | |
| contractRevision int | ||
| attestationDryRun bool | ||
| workflowName string | ||
| projectName string | ||
| ) | ||
|
|
||
| cmd := &cobra.Command{ | ||
|
|
@@ -37,14 +38,14 @@ func newAttestationInitCmd() *cobra.Command { | |
| Annotations: map[string]string{ | ||
| useAPIToken: "true", | ||
| }, | ||
| PreRunE: func(cmd *cobra.Command, args []string) error { | ||
| PreRunE: func(_ *cobra.Command, _ []string) error { | ||
| if workflowName == "" { | ||
| return errors.New("workflow name is required, set it via --name flag") | ||
| return errors.New("workflow name is required, set it via --workflow flag") | ||
| } | ||
|
|
||
| return nil | ||
| }, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| RunE: func(cmd *cobra.Command, _ []string) error { | ||
| a, err := action.NewAttestationInit( | ||
| &action.AttestationInitOpts{ | ||
| ActionsOpts: actionOpts, | ||
|
|
@@ -58,7 +59,7 @@ func newAttestationInitCmd() *cobra.Command { | |
| } | ||
|
|
||
| // Initialize it | ||
| attestationID, err := a.Run(cmd.Context(), contractRevision, workflowName) | ||
| attestationID, err := a.Run(cmd.Context(), contractRevision, projectName, workflowName) | ||
| if err != nil { | ||
| if errors.Is(err, action.ErrAttestationAlreadyExist) { | ||
| return err | ||
|
|
@@ -86,7 +87,11 @@ func newAttestationInitCmd() *cobra.Command { | |
| logger.Info().Msg("The attestation is being crafted in dry-run mode. It will not get stored once rendered") | ||
| } | ||
|
|
||
| return encodeOutput(res, simpleStatusTable) | ||
| if projectName == "" { | ||
| logger.Warn().Msg("DEPRECATION WARNING: --project not set, this will be required in the near future") | ||
| } | ||
|
|
||
| return encodeOutput(res, fullStatusTable) | ||
| }, | ||
| } | ||
|
|
||
|
|
@@ -96,10 +101,10 @@ func newAttestationInitCmd() *cobra.Command { | |
| cmd.Flags().IntVar(&contractRevision, "contract-revision", 0, "revision of the contract to retrieve, \"latest\" by default") | ||
| cmd.Flags().BoolVar(&useAttestationRemoteState, "remote-state", false, "Store the attestation state remotely") | ||
|
|
||
| // workflow-name has been replaced by --name flag | ||
| cmd.Flags().StringVar(&workflowName, "workflow-name", "", "name of the workflow to run the attestation") | ||
| cobra.CheckErr(cmd.Flags().MarkHidden("workflow-name")) | ||
| // name has been replaced by --workflow flag | ||
| cmd.Flags().StringVar(&workflowName, "workflow", "", "name of the workflow to run the attestation") | ||
| cmd.Flags().StringVar(&workflowName, "name", "", "name of the workflow to run the attestation") | ||
|
|
||
| cobra.CheckErr(cmd.Flags().MarkDeprecated("name", "please use --workflow instead")) | ||
| cmd.Flags().StringVar(&projectName, "project", "", "name of the project of this workflow") | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. project is not yet fully enforced in the attestation init |
||
| return cmd | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -81,8 +81,8 @@ func attestationStatusTableOutput(status *action.AttestationStatusResult, full b | |
| gt.AppendSeparator() | ||
| meta := status.WorkflowMeta | ||
| gt.AppendRow(table.Row{"Attestation ID", status.AttestationID}) | ||
| gt.AppendRow(table.Row{"Organization", meta.Organization}) | ||
| gt.AppendRow(table.Row{"Name", meta.Name}) | ||
| gt.AppendRow(table.Row{"Team", meta.Team}) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we removing
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really, but I see no value in showing it, not sure |
||
| gt.AppendRow(table.Row{"Project", meta.Project}) | ||
| gt.AppendRow(table.Row{"Contract Revision", meta.ContractRevision}) | ||
| if status.RunnerContext.JobURL != "" { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,15 +21,15 @@ import ( | |
| ) | ||
|
|
||
| func newWorkflowDeleteCmd() *cobra.Command { | ||
| var name string | ||
| var name, projectName string | ||
|
|
||
| cmd := &cobra.Command{ | ||
| Use: "delete", | ||
| Short: "Delete an existing workflow", | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| err := action.NewWorkflowDelete(actionOpts).Run(name) | ||
| err := action.NewWorkflowDelete(actionOpts).Run(name, projectName) | ||
| if err == nil { | ||
| logger.Info().Msg("Workload deleted!") | ||
| logger.Info().Msg("Workflow deleted!") | ||
| } | ||
| return err | ||
| }, | ||
|
|
@@ -38,5 +38,8 @@ func newWorkflowDeleteCmd() *cobra.Command { | |
| cmd.Flags().StringVar(&name, "name", "", "workflow name") | ||
| cobra.CheckErr(cmd.MarkFlagRequired("name")) | ||
|
|
||
| cmd.Flags().StringVar(&projectName, "project", "", "project name") | ||
| cobra.CheckErr(cmd.MarkFlagRequired("project")) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but it is in the rest of the APIs. |
||
|
|
||
| return cmd | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got some feedback from @juamedgod that
--namewas confusing since it seemed the name of the attestation itself.So I have changed it to
workflowwhile keeping the existing--nameas deprecated