-
Notifications
You must be signed in to change notification settings - Fork 110
feat(cli): make quickstart catalog repository configurable via --catalog-repo flag #6364
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
base: main
Are you sure you want to change the base?
Changes from all commits
ddf58be
9971fda
726c1ef
b1766e1
05441ff
e9718ac
0891c99
ece5e51
55781c9
a061c15
e726e90
98da8a5
30279ec
2c0dc21
3e604df
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 | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,11 +7,12 @@ package quickstart | |||||||||||
|
|
||||||||||||
| import ( | ||||||||||||
| "context" | ||||||||||||
| "embed" | ||||||||||||
| "fmt" | ||||||||||||
| "os" | ||||||||||||
| "strings" | ||||||||||||
|
|
||||||||||||
| git "github.com/go-git/go-git/v5" | ||||||||||||
| "github.com/go-git/go-git/v5/storage/memory" | ||||||||||||
| "github.com/spf13/cobra" | ||||||||||||
| "github.com/spf13/viper" | ||||||||||||
| "google.golang.org/grpc" | ||||||||||||
|
|
@@ -20,13 +21,12 @@ import ( | |||||||||||
|
|
||||||||||||
| "github.com/mindersec/minder/cmd/cli/app" | ||||||||||||
| "github.com/mindersec/minder/cmd/cli/app/auth" | ||||||||||||
| "github.com/mindersec/minder/cmd/cli/app/profile" | ||||||||||||
| minderprov "github.com/mindersec/minder/cmd/cli/app/provider" | ||||||||||||
| "github.com/mindersec/minder/cmd/cli/app/repo" | ||||||||||||
| internalcli "github.com/mindersec/minder/internal/cli" | ||||||||||||
| ghclient "github.com/mindersec/minder/internal/providers/github/clients" | ||||||||||||
| "github.com/mindersec/minder/internal/util/cli" | ||||||||||||
| minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" | ||||||||||||
| "github.com/mindersec/minder/pkg/profiles" | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| const ( | ||||||||||||
|
|
@@ -128,9 +128,6 @@ In case you have registered new repositories during this flow, the profile will | |||||||||||
| ` | ||||||||||||
| ) | ||||||||||||
|
|
||||||||||||
| //go:embed embed* | ||||||||||||
| var content embed.FS | ||||||||||||
|
|
||||||||||||
|
Comment on lines
-131
to
-133
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. Can we remove the |
||||||||||||
| var cmd = &cobra.Command{ | ||||||||||||
| Use: "quickstart", | ||||||||||||
| Short: "Quickstart minder", | ||||||||||||
|
|
@@ -249,24 +246,35 @@ func quickstartCommand( | |||||||||||
| r := fmt.Sprintf("%s/%s", result.Owner, result.Name) | ||||||||||||
| registeredRepos = append(registeredRepos, r) | ||||||||||||
| } | ||||||||||||
| repoURL, err := cmd.Flags().GetString("catalog-repo") | ||||||||||||
| if err != nil { | ||||||||||||
| return err | ||||||||||||
| } | ||||||||||||
| if repoURL == "" { | ||||||||||||
| repoURL = defaultQuickstartCatalogRepoURL | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return loadCatalog(cmd, ruleClient, profileClient, provider, project, registeredRepos) | ||||||||||||
| } | ||||||||||||
| clonedRepo, err := git.Clone(memory.NewStorage(), nil, &git.CloneOptions{ | ||||||||||||
| URL: repoURL, | ||||||||||||
| Depth: 1, | ||||||||||||
| }) | ||||||||||||
| if err != nil { | ||||||||||||
| return fmt.Errorf("failed to load catalog repo: %w", err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // loadCatalog loads and applies the quickstart rule type and profile catalog | ||||||||||||
| // using the same prompts and flow as the inline implementation. | ||||||||||||
| // | ||||||||||||
| //nolint:gocyclo // kept as a straight extraction to preserve behavior | ||||||||||||
| func loadCatalog( | ||||||||||||
| cmd *cobra.Command, | ||||||||||||
| ruleClient minderv1.RuleTypeServiceClient, | ||||||||||||
| profileClient minderv1.ProfileServiceClient, | ||||||||||||
| provider string, | ||||||||||||
| project string, | ||||||||||||
| registeredRepos []string, | ||||||||||||
| ) error { | ||||||||||||
| worktree, err := clonedRepo.Worktree() | ||||||||||||
| if err != nil { | ||||||||||||
| return fmt.Errorf("failed to load catalog repo: %w", err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| catalog, err := internalcli.LoadCatalogFromFS(worktree.Filesystem, cmd.Printf) | ||||||||||||
| if err != nil { | ||||||||||||
| return fmt.Errorf("failed to load catalog: %w", err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Now prompt user AFTER validation | ||||||||||||
| // Step 3 - Confirm rule type creation | ||||||||||||
| yes := cli.PrintYesNoPrompt(cmd, | ||||||||||||
| yes = cli.PrintYesNoPrompt(cmd, | ||||||||||||
| stepPromptMsgRuleType, | ||||||||||||
| "Proceed?", | ||||||||||||
| "Quickstart operation cancelled.", | ||||||||||||
|
|
@@ -275,116 +283,147 @@ func loadCatalog( | |||||||||||
| return nil | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Creating the rule type | ||||||||||||
| cmd.Println("Creating rule type...") | ||||||||||||
|
|
||||||||||||
| // Load the rule type from the embedded file system | ||||||||||||
| reader, err := content.Open("embed/secret_scanning.yaml") | ||||||||||||
| if err != nil { | ||||||||||||
| return cli.MessageAndError("error opening rule type", err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| rt := &minderv1.RuleType{} | ||||||||||||
|
|
||||||||||||
| if err := minderv1.ParseResource(reader, rt); err != nil { | ||||||||||||
| return cli.MessageAndError("error parsing rule type", err) | ||||||||||||
| // Step 4 - Confirm profile creation with context | ||||||||||||
| yes = cli.PrintYesNoPrompt(cmd, | ||||||||||||
| fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos, "\n")), | ||||||||||||
| "Proceed?", | ||||||||||||
| "Quickstart operation cancelled.", | ||||||||||||
| true) | ||||||||||||
| if !yes { | ||||||||||||
| return nil | ||||||||||||
|
Comment on lines
277
to
+293
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. It's a little odd to have two "yes/no" prompts right in a row when nothing has happened between them. Can we coalesce them into one prompt? |
||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if rt.Context == nil { | ||||||||||||
| rt.Context = &minderv1.Context{} | ||||||||||||
| } | ||||||||||||
| // Apply all resources (transactional flow) | ||||||||||||
|
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. Include the transactional behavior in the method godoc, not at the call site. |
||||||||||||
| return applyCatalog(cmd, ruleClient, profileClient, catalog, project) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| rt.Context = &minderv1.Context{ | ||||||||||||
| Provider: &provider, | ||||||||||||
| Project: &project, | ||||||||||||
| } | ||||||||||||
| const ( | ||||||||||||
| defaultQuickstartCatalogRepoURL = "https://github.com/mindersec/minder-rules-and-profiles" | ||||||||||||
| ) | ||||||||||||
|
Comment on lines
+300
to
+302
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. Slight preference to have constants at the top of the file, or at least before use. |
||||||||||||
|
|
||||||||||||
| // New context so we don't time out between steps | ||||||||||||
|
sachin9058 marked this conversation as resolved.
|
||||||||||||
| // applyCatalog creates all rule types and profiles from the catalog via gRPC services. | ||||||||||||
| // | ||||||||||||
| // This function is called AFTER user confirmation and validation. It creates all | ||||||||||||
| // resources in the catalog. If any creation fails, the error is returned immediately. | ||||||||||||
|
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. This says "If any creation fails, the error is returned immediately", but this actually seems to try to clean up in some cases. I'm not sure whether cleanup or "handle existing resources" is a better approach, but we should line up this doc with the "transactional flow" comment above. |
||||||||||||
| // The function always prints a summary of created resources, whether new or already | ||||||||||||
| // existing. | ||||||||||||
| func applyCatalog( | ||||||||||||
| cmd *cobra.Command, | ||||||||||||
| ruleClient minderv1.RuleTypeServiceClient, | ||||||||||||
| profileClient minderv1.ProfileServiceClient, | ||||||||||||
| catalog *internalcli.Catalog, | ||||||||||||
| project string, | ||||||||||||
| ) error { | ||||||||||||
| ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) | ||||||||||||
| defer cancel() | ||||||||||||
|
|
||||||||||||
| // Create the rule type in minder | ||||||||||||
| _, err = ruleClient.CreateRuleType(ctx, &minderv1.CreateRuleTypeRequest{ | ||||||||||||
| RuleType: rt, | ||||||||||||
| }) | ||||||||||||
| if err != nil { | ||||||||||||
| if st, ok := status.FromError(err); ok { | ||||||||||||
| if st.Code() != codes.AlreadyExists { | ||||||||||||
| return fmt.Errorf("error creating rule type from: %w", err) | ||||||||||||
| } | ||||||||||||
| cmd.Println("Rule type secret_scanning already exists") | ||||||||||||
| } else { | ||||||||||||
| return cli.MessageAndError("error creating rule type", err) | ||||||||||||
| } | ||||||||||||
| projectContext := &minderv1.Context{ | ||||||||||||
| Project: &project, | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Step 4 - Confirm profile creation | ||||||||||||
| yes = cli.PrintYesNoPrompt(cmd, | ||||||||||||
| fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos[:], "\n")), | ||||||||||||
| "Proceed?", | ||||||||||||
| "Quickstart operation cancelled.", | ||||||||||||
| true) | ||||||||||||
| if !yes { | ||||||||||||
| return nil | ||||||||||||
| result := applyCatalogResult{ | ||||||||||||
| createdRuleTypes: make([]string, 0, len(catalog.RuleTypes)), | ||||||||||||
| createdProfiles: make([]string, 0, len(catalog.Profiles)), | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Creating the profile | ||||||||||||
| cmd.Println("Creating profile...") | ||||||||||||
| reader, err = content.Open("embed/profile.yaml") | ||||||||||||
| if err != nil { | ||||||||||||
| return cli.MessageAndError("error opening profile", err) | ||||||||||||
| if err := createCatalogRuleTypes(ctx, cmd, ruleClient, catalog.RuleTypes, projectContext, &result); err != nil { | ||||||||||||
| rollbackCatalogResources(ctx, ruleClient, profileClient, result) | ||||||||||||
|
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. This makes sense given two cases; for larger flows, I'd use a func DoThing() (err error) {
created := map[string]error
defer func() {
if error != nil {
for k, _ := range created {
cleanup(k) // not using stored error
}
}
}()
// do stuff and store in created. Be careful not to shadow err in "if" blocks, i.e.
err := makeStuff(created)
if err != nil { // using "if err :=...; err != nil" here would shadow the outer err
return err
}
} |
||||||||||||
| return err | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Load the profile from the embedded file system | ||||||||||||
| p, err := profiles.ParseYAML(reader) | ||||||||||||
| if err != nil { | ||||||||||||
| return cli.MessageAndError("error parsing profile", err) | ||||||||||||
| if err := createCatalogProfiles(ctx, cmd, profileClient, catalog.Profiles, projectContext, &result); err != nil { | ||||||||||||
| rollbackCatalogResources(ctx, ruleClient, profileClient, result) | ||||||||||||
| return err | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| if p.Context == nil { | ||||||||||||
| p.Context = &minderv1.Context{} | ||||||||||||
| printCatalogSummary(cmd, result) | ||||||||||||
| return nil | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| type applyCatalogResult struct { | ||||||||||||
| createdRuleTypes []string | ||||||||||||
| createdProfiles []string | ||||||||||||
| seenExistingProfile bool | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+342
to
+346
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. Again, prefer to declare structs before usage, so someone reading from the top of file can understand what's in them before they are used. |
||||||||||||
|
|
||||||||||||
| func rollbackCatalogResources( | ||||||||||||
| ctx context.Context, | ||||||||||||
| ruleClient minderv1.RuleTypeServiceClient, | ||||||||||||
| profileClient minderv1.ProfileServiceClient, | ||||||||||||
| result applyCatalogResult, | ||||||||||||
| ) { | ||||||||||||
| for _, profileID := range result.createdProfiles { | ||||||||||||
| _, _ = profileClient.DeleteProfile(ctx, &minderv1.DeleteProfileRequest{Id: profileID}) | ||||||||||||
| } | ||||||||||||
| for _, ruleTypeID := range result.createdRuleTypes { | ||||||||||||
| _, _ = ruleClient.DeleteRuleType(ctx, &minderv1.DeleteRuleTypeRequest{Id: ruleTypeID}) | ||||||||||||
| } | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func createCatalogRuleTypes( | ||||||||||||
| ctx context.Context, | ||||||||||||
| cmd *cobra.Command, | ||||||||||||
| ruleClient minderv1.RuleTypeServiceClient, | ||||||||||||
| ruleTypes []*minderv1.RuleType, | ||||||||||||
| projectContext *minderv1.Context, | ||||||||||||
| result *applyCatalogResult, | ||||||||||||
| ) error { | ||||||||||||
| for _, ruleType := range ruleTypes { | ||||||||||||
| ruleType.Context = projectContext | ||||||||||||
|
|
||||||||||||
| p.Context = &minderv1.Context{ | ||||||||||||
| Provider: &provider, | ||||||||||||
| Project: &project, | ||||||||||||
| cmd.Printf("Creating rule type %s...\n", ruleType.GetName()) | ||||||||||||
| resp, err := ruleClient.CreateRuleType(ctx, &minderv1.CreateRuleTypeRequest{RuleType: ruleType}) | ||||||||||||
| if err != nil { | ||||||||||||
| if st, ok := status.FromError(err); ok && st.Code() == codes.AlreadyExists { | ||||||||||||
| cmd.Printf("Rule type %s already exists\n", ruleType.GetName()) | ||||||||||||
| continue | ||||||||||||
| } | ||||||||||||
| return fmt.Errorf("error creating rule type %s: %w", ruleType.GetName(), err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| name := resp.GetRuleType().GetName() | ||||||||||||
| if name == "" { | ||||||||||||
| name = ruleType.GetName() | ||||||||||||
| } | ||||||||||||
|
Comment on lines
+383
to
+386
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. Slight preference for
Suggested change
I'm also not sure how the empty string from the server could happen. Is this something we've seen? |
||||||||||||
| result.createdRuleTypes = append(result.createdRuleTypes, name) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // New context so we don't time out between steps | ||||||||||||
| ctx, cancel = getQuickstartContext(cmd.Context(), viper.GetViper()) | ||||||||||||
| defer cancel() | ||||||||||||
| return nil | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| alreadyExists := false | ||||||||||||
| // Create the profile in minder | ||||||||||||
| resp, err := profileClient.CreateProfile(ctx, &minderv1.CreateProfileRequest{ | ||||||||||||
| Profile: p, | ||||||||||||
| }) | ||||||||||||
| if err != nil { | ||||||||||||
| if st, ok := status.FromError(err); ok { | ||||||||||||
| if st.Code() != codes.AlreadyExists { | ||||||||||||
| return cli.MessageAndError("error creating profile", err) | ||||||||||||
| func createCatalogProfiles( | ||||||||||||
| ctx context.Context, | ||||||||||||
| cmd *cobra.Command, | ||||||||||||
| profileClient minderv1.ProfileServiceClient, | ||||||||||||
| loadedProfiles []*minderv1.Profile, | ||||||||||||
| projectContext *minderv1.Context, | ||||||||||||
| result *applyCatalogResult, | ||||||||||||
| ) error { | ||||||||||||
| for _, profileResource := range loadedProfiles { | ||||||||||||
| profileResource.Context = projectContext | ||||||||||||
|
|
||||||||||||
| cmd.Printf("Creating profile %s...\n", profileResource.GetName()) | ||||||||||||
| resp, err := profileClient.CreateProfile(ctx, &minderv1.CreateProfileRequest{Profile: profileResource}) | ||||||||||||
| if err != nil { | ||||||||||||
| if st, ok := status.FromError(err); ok && st.Code() == codes.AlreadyExists { | ||||||||||||
| cmd.Printf("Profile %s already exists\n", profileResource.GetName()) | ||||||||||||
| result.seenExistingProfile = true | ||||||||||||
| continue | ||||||||||||
| } | ||||||||||||
| alreadyExists = true | ||||||||||||
| } else { | ||||||||||||
| return cli.MessageAndError("error creating profile", err) | ||||||||||||
| return fmt.Errorf("error creating profile %s: %w", profileResource.GetName(), err) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| result.createdProfiles = append(result.createdProfiles, resp.GetProfile().GetId()) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| // Finish - Confirm profile creation | ||||||||||||
| if alreadyExists { | ||||||||||||
| // Print the "profile already exists" message | ||||||||||||
| return nil | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func printCatalogSummary(cmd *cobra.Command, result applyCatalogResult) { | ||||||||||||
| if result.seenExistingProfile { | ||||||||||||
| cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishExisting + stepPromptMsgFinishBase)) | ||||||||||||
| } else { | ||||||||||||
| // Print the "profile created" message | ||||||||||||
| cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishOK + stepPromptMsgFinishBase)) | ||||||||||||
| // Print the profile create result table | ||||||||||||
| cmd.Println("Profile details (minder profile list):") | ||||||||||||
| table := profile.NewProfileRulesTable(cmd.OutOrStdout()) | ||||||||||||
| profile.RenderProfileRulesTable(resp.GetProfile(), table) | ||||||||||||
| table.Render() | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| return nil | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| func init() { | ||||||||||||
|
|
@@ -394,6 +433,7 @@ func init() { | |||||||||||
| cmd.Flags().StringP("project", "j", "", "ID of the project") | ||||||||||||
| cmd.Flags().StringP("token", "t", "", "Personal Access Token (PAT) to use for enrollment") | ||||||||||||
| cmd.Flags().StringP("owner", "o", "", "Owner to filter on for provider resources") | ||||||||||||
| cmd.Flags().String("catalog-repo", "", "Repository URL to load quickstart catalog from") | ||||||||||||
| // Bind flags | ||||||||||||
| if err := viper.BindPFlag("token", cmd.Flags().Lookup("token")); err != nil { | ||||||||||||
| cmd.Printf("error: %s", err) | ||||||||||||
|
|
||||||||||||
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.
Why not put this in
internal/util/cli, which we import 2 lines later? Having two different directories for CLI-implementation-related modules is confusing to future authors.