From ddf58bea9face24ad442663563bc1739539319b3 Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Tue, 14 Apr 2026 18:23:24 +0530 Subject: [PATCH 1/9] feat(cli): make quickstart catalog repository configurable via --catalog-repo flag --- cmd/cli/app/quickstart/quickstart.go | 221 ++++++++++++++++++++++++--- 1 file changed, 196 insertions(+), 25 deletions(-) diff --git a/cmd/cli/app/quickstart/quickstart.go b/cmd/cli/app/quickstart/quickstart.go index e253fbcd27..7ec32d7703 100644 --- a/cmd/cli/app/quickstart/quickstart.go +++ b/cmd/cli/app/quickstart/quickstart.go @@ -23,6 +23,7 @@ import ( "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" + internalrepo "github.com/mindersec/minder/cmd/cli/internal/repo" 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" @@ -249,9 +250,46 @@ 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, repoURL) +} + +const ( + quickstartRuleTypeFilePath = "rule-types/github/secret_scanning.yaml" + quickstartProfileFilePath = "profiles/github/profile.yaml" + defaultQuickstartCatalogRepoURL = "https://github.com/mindersec/minder-rules-and-profiles" +) +// loadCatalog drives the catalog portion of the quickstart flow. +// +// It first asks the user to confirm creation of the initial quickstart +// resources (the secret_scanning rule type and its profile), then attempts +// to source those resources from the configured catalog repository using +// loadCatalogFromRepo. +// +// If cloning the catalog repository or reading/parsing its files fails for +// any reason, loadCatalog prints a warning and transparently falls back to +// runExistingFlow, which uses the embedded quickstart YAML files shipped +// with the CLI. This preserves the original quickstart behavior while +// preferring the up-to-date remote catalog when available. +func loadCatalog( + cmd *cobra.Command, + ruleClient minderv1.RuleTypeServiceClient, + profileClient minderv1.ProfileServiceClient, + provider string, + project string, + registeredRepos []string, + repoURL string, +) error { // Step 3 - Confirm rule type creation - yes = cli.PrintYesNoPrompt(cmd, + yes := cli.PrintYesNoPrompt(cmd, stepPromptMsgRuleType, "Proceed?", "Quickstart operation cancelled.", @@ -260,17 +298,163 @@ func quickstartCommand( return nil } - // Creating the rule type - cmd.Println("Creating rule type...") + if err := loadCatalogFromRepo(cmd, ruleClient, profileClient, provider, project, registeredRepos, repoURL); err != nil { + cmd.Printf("Warning: failed to load quickstart catalog from %s: %v\n", repoURL, err) + cmd.Printf("Falling back to embedded quickstart catalog.\n") + return runExistingFlow(cmd, ruleClient, profileClient, provider, project, registeredRepos) + } + + return nil +} + +// loadCatalogFromRepo loads the quickstart rule type and profile directly +// from the configured Git repository. +// +// The function clones the catalog repository in memory, opens the +// quickstart rule type and profile YAML files from the in-memory +// filesystem, parses them into protobuf/CLI profile structures, applies +// the current provider/project context, and then creates the resources via +// the RuleType and Profile gRPC services. +// +// It mirrors the prompts and output of the original quickstart flow while +// sourcing the definitions from a remote catalog instead of the embedded +// YAML. Any error is propagated to the caller so that a higher-level +// fallback (to the embedded flow) can be applied. +func loadCatalogFromRepo( + cmd *cobra.Command, + ruleClient minderv1.RuleTypeServiceClient, + profileClient minderv1.ProfileServiceClient, + provider string, + project string, + registeredRepos []string, + repoURL string, +) error { + catalogRepo, err := internalrepo.CloneInMemory(repoURL) + if err != nil { + return fmt.Errorf("failed to clone catalog repository: %w", err) + } + + worktree, err := catalogRepo.Worktree() + if err != nil { + return fmt.Errorf("failed to get worktree: %w", err) + } - // Load the rule type from the embedded file system + fs := worktree.Filesystem + + rtReader, err := fs.Open(quickstartRuleTypeFilePath) + if err != nil { + return fmt.Errorf("failed to open rule type file: %w", err) + } + defer rtReader.Close() + + rt := &minderv1.RuleType{} + if err := minderv1.ParseResource(rtReader, rt); err != nil { + return fmt.Errorf("failed to parse rule type: %w", err) + } + + rt.Context = &minderv1.Context{ + Provider: &provider, + Project: &project, + } + + ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) + defer cancel() + + cmd.Printf("Creating rule type from remote catalog...\n") + _, 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 remote catalog: %w", err) + } + cmd.Println("Rule type secret_scanning already exists") + } else { + return cli.MessageAndError("error creating rule type", err) + } + } + + yes := cli.PrintYesNoPrompt(cmd, + fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos, "\n")), + "Proceed?", + "Quickstart operation cancelled.", + true) + if !yes { + return nil + } + + cmd.Printf("Creating profile from remote catalog...\n") + profileReader, err := fs.Open(quickstartProfileFilePath) + if err != nil { + return fmt.Errorf("failed to open profile file: %w", err) + } + defer profileReader.Close() + + p, err := profiles.ParseYAML(profileReader) + if err != nil { + return fmt.Errorf("failed to parse profile: %w", err) + } + + p.Context = &minderv1.Context{ + Provider: &provider, + Project: &project, + } + + ctx, cancel = getQuickstartContext(cmd.Context(), viper.GetViper()) + defer cancel() + + alreadyExists := false + 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) + } + alreadyExists = true + } else { + return cli.MessageAndError("error creating profile", err) + } + } + + if alreadyExists { + cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishExisting + stepPromptMsgFinishBase)) + } else { + cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishOK + stepPromptMsgFinishBase)) + cmd.Println("Profile details (minder profile list):") + table := profile.NewProfileRulesTable(cmd.OutOrStdout()) + profile.RenderProfileRulesTable(resp.GetProfile(), table) + table.Render() + } + + return nil +} + +// runExistingFlow contains the original quickstart catalog logic that uses +// the embedded secret_scanning rule type and profile YAML files. +// +// This function is used as a safe fallback when loading the catalog from +// the configured repository fails. It recreates the previous Step 3 and +// Step 4 behavior by: +// - reading secret_scanning.yaml and profile.yaml from the embedded FS, +// - parsing them into the appropriate rule type and profile structures, +// - applying the current provider/project context, and +// - creating the resources via the corresponding gRPC services, including +// handling AlreadyExists responses and printing the final banners and +// profile details table. +func runExistingFlow( + cmd *cobra.Command, + ruleClient minderv1.RuleTypeServiceClient, + profileClient minderv1.ProfileServiceClient, + provider string, + project string, + registeredRepos []string, +) error { + cmd.Println("Creating rule type...") 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) } @@ -284,14 +468,10 @@ func quickstartCommand( Project: &project, } - // New context so we don't time out between steps - ctx, cancel = getQuickstartContext(cmd.Context(), viper.GetViper()) + ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) defer cancel() - // Create the rule type in minder - _, err = ruleClient.CreateRuleType(ctx, &minderv1.CreateRuleTypeRequest{ - RuleType: rt, - }) + _, err = ruleClient.CreateRuleType(ctx, &minderv1.CreateRuleTypeRequest{RuleType: rt}) if err != nil { if st, ok := status.FromError(err); ok { if st.Code() != codes.AlreadyExists { @@ -303,9 +483,8 @@ func quickstartCommand( } } - // Step 4 - Confirm profile creation - yes = cli.PrintYesNoPrompt(cmd, - fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos[:], "\n")), + yes := cli.PrintYesNoPrompt(cmd, + fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos, "\n")), "Proceed?", "Quickstart operation cancelled.", true) @@ -313,14 +492,12 @@ func quickstartCommand( return nil } - // Creating the profile cmd.Println("Creating profile...") reader, err = content.Open("embed/profile.yaml") if err != nil { return cli.MessageAndError("error opening profile", err) } - // Load the profile from the embedded file system p, err := profiles.ParseYAML(reader) if err != nil { return cli.MessageAndError("error parsing profile", err) @@ -335,15 +512,11 @@ func quickstartCommand( Project: &project, } - // New context so we don't time out between steps ctx, cancel = getQuickstartContext(cmd.Context(), viper.GetViper()) defer cancel() alreadyExists := false - // Create the profile in minder - resp, err := profileClient.CreateProfile(ctx, &minderv1.CreateProfileRequest{ - Profile: p, - }) + resp, err := profileClient.CreateProfile(ctx, &minderv1.CreateProfileRequest{Profile: p}) if err != nil { if st, ok := status.FromError(err); ok { if st.Code() != codes.AlreadyExists { @@ -355,19 +528,16 @@ func quickstartCommand( } } - // Finish - Confirm profile creation if alreadyExists { - // Print the "profile already exists" message 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 } @@ -378,6 +548,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) From 9971fdacabf477f6cb679e483d749b8bc30749ca Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Tue, 14 Apr 2026 20:12:10 +0530 Subject: [PATCH 2/9] lint-fixed --- cmd/cli/app/quickstart/quickstart.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/cli/app/quickstart/quickstart.go b/cmd/cli/app/quickstart/quickstart.go index 7ec32d7703..5194100276 100644 --- a/cmd/cli/app/quickstart/quickstart.go +++ b/cmd/cli/app/quickstart/quickstart.go @@ -12,6 +12,8 @@ import ( "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" @@ -23,7 +25,6 @@ import ( "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" - internalrepo "github.com/mindersec/minder/cmd/cli/internal/repo" 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" @@ -329,7 +330,7 @@ func loadCatalogFromRepo( registeredRepos []string, repoURL string, ) error { - catalogRepo, err := internalrepo.CloneInMemory(repoURL) + catalogRepo, err := git.Clone(memory.NewStorage(), nil, &git.CloneOptions{URL: repoURL}) if err != nil { return fmt.Errorf("failed to clone catalog repository: %w", err) } From 726c1eff8b05c78fc91e932cbf4d52839e7eedfc Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Tue, 14 Apr 2026 23:15:51 +0530 Subject: [PATCH 3/9] refactor(cli): move catalog clone logic to internal/cli and decouple quickstart from clone helper --- cmd/cli/app/quickstart/quickstart.go | 12 ++-------- internal/cli/catalog.go | 33 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 10 deletions(-) create mode 100644 internal/cli/catalog.go diff --git a/cmd/cli/app/quickstart/quickstart.go b/cmd/cli/app/quickstart/quickstart.go index 5194100276..d2a22bbc17 100644 --- a/cmd/cli/app/quickstart/quickstart.go +++ b/cmd/cli/app/quickstart/quickstart.go @@ -12,8 +12,6 @@ import ( "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" @@ -25,6 +23,7 @@ import ( "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" + cliintern "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" @@ -330,18 +329,11 @@ func loadCatalogFromRepo( registeredRepos []string, repoURL string, ) error { - catalogRepo, err := git.Clone(memory.NewStorage(), nil, &git.CloneOptions{URL: repoURL}) + fs, err := cliintern.CloneRepoFilesystem(repoURL) if err != nil { return fmt.Errorf("failed to clone catalog repository: %w", err) } - worktree, err := catalogRepo.Worktree() - if err != nil { - return fmt.Errorf("failed to get worktree: %w", err) - } - - fs := worktree.Filesystem - rtReader, err := fs.Open(quickstartRuleTypeFilePath) if err != nil { return fmt.Errorf("failed to open rule type file: %w", err) diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go new file mode 100644 index 0000000000..be398edfc4 --- /dev/null +++ b/internal/cli/catalog.go @@ -0,0 +1,33 @@ +// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +// Package cli contains reusable CLI helper logic for catalog operations +package cli + +import ( + "fmt" + + "github.com/go-git/go-billy/v5" + git "github.com/go-git/go-git/v5" + "github.com/go-git/go-git/v5/storage/memory" +) + +// CloneRepoFilesystem clones the given Git repository URL into an in-memory +// storage and returns its filesystem. Callers can use the returned +// filesystem to open files and traverse directories without touching disk. +func CloneRepoFilesystem(repoURL string) (billy.Filesystem, error) { + repo, err := git.Clone(memory.NewStorage(), nil, &git.CloneOptions{ + URL: repoURL, + Depth: 1, + }) + if err != nil { + return nil, fmt.Errorf("failed to clone repository %s: %w", repoURL, err) + } + + worktree, err := repo.Worktree() + if err != nil { + return nil, fmt.Errorf("failed to get worktree for %s: %w", repoURL, err) + } + + return worktree.Filesystem, nil +} From b1766e1dca4e5c133e513a6b9782d0e7f39b17ae Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Thu, 23 Apr 2026 02:59:08 +0530 Subject: [PATCH 4/9] feat(cli): implement filesystem-driven catalog loading and validation for quickstart Signed-off-by: Sachin Kumar --- cmd/cli/app/quickstart/quickstart.go | 486 ++++++++++++++++----------- internal/cli/catalog.go | 33 -- 2 files changed, 288 insertions(+), 231 deletions(-) delete mode 100644 internal/cli/catalog.go diff --git a/cmd/cli/app/quickstart/quickstart.go b/cmd/cli/app/quickstart/quickstart.go index d2a22bbc17..a071dbf1c6 100644 --- a/cmd/cli/app/quickstart/quickstart.go +++ b/cmd/cli/app/quickstart/quickstart.go @@ -7,11 +7,15 @@ package quickstart import ( "context" - "embed" "fmt" "os" + "path/filepath" + "sort" "strings" + "github.com/go-git/go-billy/v5" + 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" @@ -23,7 +27,6 @@ import ( "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" - cliintern "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" @@ -129,9 +132,6 @@ In case you have registered new repositories during this flow, the profile will ` ) -//go:embed embed* -var content embed.FS - var cmd = &cobra.Command{ Use: "quickstart", Short: "Quickstart minder", @@ -258,38 +258,27 @@ func quickstartCommand( repoURL = defaultQuickstartCatalogRepoURL } - return loadCatalog(cmd, ruleClient, profileClient, provider, project, registeredRepos, repoURL) -} + 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) + } -const ( - quickstartRuleTypeFilePath = "rule-types/github/secret_scanning.yaml" - quickstartProfileFilePath = "profiles/github/profile.yaml" - defaultQuickstartCatalogRepoURL = "https://github.com/mindersec/minder-rules-and-profiles" -) + worktree, err := clonedRepo.Worktree() + if err != nil { + return fmt.Errorf("failed to load catalog repo: %w", err) + } -// loadCatalog drives the catalog portion of the quickstart flow. -// -// It first asks the user to confirm creation of the initial quickstart -// resources (the secret_scanning rule type and its profile), then attempts -// to source those resources from the configured catalog repository using -// loadCatalogFromRepo. -// -// If cloning the catalog repository or reading/parsing its files fails for -// any reason, loadCatalog prints a warning and transparently falls back to -// runExistingFlow, which uses the embedded quickstart YAML files shipped -// with the CLI. This preserves the original quickstart behavior while -// preferring the up-to-date remote catalog when available. -func loadCatalog( - cmd *cobra.Command, - ruleClient minderv1.RuleTypeServiceClient, - profileClient minderv1.ProfileServiceClient, - provider string, - project string, - registeredRepos []string, - repoURL string, -) error { + catalog, err := LoadCatalogFromFS(worktree.Filesystem) + 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.", @@ -298,240 +287,341 @@ func loadCatalog( return nil } - if err := loadCatalogFromRepo(cmd, ruleClient, profileClient, provider, project, registeredRepos, repoURL); err != nil { - cmd.Printf("Warning: failed to load quickstart catalog from %s: %v\n", repoURL, err) - cmd.Printf("Falling back to embedded quickstart catalog.\n") - return runExistingFlow(cmd, ruleClient, profileClient, provider, project, registeredRepos) + // 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 } - return nil + // Apply all resources (transactional flow) + return applyCatalog(cmd, ruleClient, profileClient, catalog, project) } -// loadCatalogFromRepo loads the quickstart rule type and profile directly -// from the configured Git repository. -// -// The function clones the catalog repository in memory, opens the -// quickstart rule type and profile YAML files from the in-memory -// filesystem, parses them into protobuf/CLI profile structures, applies -// the current provider/project context, and then creates the resources via -// the RuleType and Profile gRPC services. +const ( + defaultQuickstartCatalogRepoURL = "https://github.com/mindersec/minder-rules-and-profiles" + catalogRuleTypesDir = "rule-types" + catalogProfilesDir = "profiles" +) + +// Catalog represents a loaded collection of rule types and profiles from a filesystem. +type Catalog struct { + RuleTypes []*minderv1.RuleType + Profiles []*minderv1.Profile +} + +// LoadCatalogFromFS loads and validates all YAML resources under the catalog directories. // -// It mirrors the prompts and output of the original quickstart flow while -// sourcing the definitions from a remote catalog instead of the embedded -// YAML. Any error is propagated to the caller so that a higher-level -// fallback (to the embedded flow) can be applied. -func loadCatalogFromRepo( - cmd *cobra.Command, - ruleClient minderv1.RuleTypeServiceClient, - profileClient minderv1.ProfileServiceClient, - provider string, - project string, - registeredRepos []string, - repoURL string, -) error { - fs, err := cliintern.CloneRepoFilesystem(repoURL) +// It recursively scans the rule-types and profiles directories, loads every YAML file, +// and verifies that each rule type referenced by each profile exists in the loaded set. +func LoadCatalogFromFS(fs billy.Filesystem) (*Catalog, error) { + ruleTypes, ruleTypeNames, err := loadRuleTypesFromFS(fs) if err != nil { - return fmt.Errorf("failed to clone catalog repository: %w", err) + return nil, err } - rtReader, err := fs.Open(quickstartRuleTypeFilePath) + loadedProfiles, err := loadProfilesFromFS(fs) if err != nil { - return fmt.Errorf("failed to open rule type file: %w", err) + return nil, err } - defer rtReader.Close() - rt := &minderv1.RuleType{} - if err := minderv1.ParseResource(rtReader, rt); err != nil { - return fmt.Errorf("failed to parse rule type: %w", err) + if err := validateCatalog(ruleTypeNames, loadedProfiles); err != nil { + return nil, err } - rt.Context = &minderv1.Context{ - Provider: &provider, - Project: &project, + return &Catalog{RuleTypes: ruleTypes, Profiles: loadedProfiles}, nil +} + +func loadRuleTypesFromFS(fs billy.Filesystem) ([]*minderv1.RuleType, map[string]struct{}, error) { + paths, err := collectYAMLFiles(fs, catalogRuleTypesDir) + if err != nil { + return nil, nil, err + } + if len(paths) == 0 { + return nil, nil, fmt.Errorf("no rule type YAML files found under %s", catalogRuleTypesDir) } - ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) - defer cancel() + // Keep load order deterministic across filesystems. + sort.Strings(paths) + ruleTypes := make([]*minderv1.RuleType, 0, len(paths)) + ruleTypeNames := make(map[string]struct{}, len(paths)) - cmd.Printf("Creating rule type from remote catalog...\n") - _, 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 remote catalog: %w", err) + for _, path := range paths { + if err := func() error { + reader, err := fs.Open(path) + if err != nil { + return fmt.Errorf("failed to open rule type file %s: %w", path, err) } - cmd.Println("Rule type secret_scanning already exists") - } else { - return cli.MessageAndError("error creating rule type", err) + defer reader.Close() + + ruleType := &minderv1.RuleType{} + if err := minderv1.ParseResource(reader, ruleType); err != nil { + return fmt.Errorf("failed to parse rule type file %s: %w", path, err) + } + if ruleType.GetName() == "" { + return fmt.Errorf("rule type file %s has no name", path) + } + if _, exists := ruleTypeNames[ruleType.GetName()]; exists { + return fmt.Errorf("duplicate rule type %q found in %s", ruleType.GetName(), path) + } + ruleTypeNames[ruleType.GetName()] = struct{}{} + ruleTypes = append(ruleTypes, ruleType) + return nil + }(); err != nil { + return nil, nil, err } } - yes := cli.PrintYesNoPrompt(cmd, - fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos, "\n")), - "Proceed?", - "Quickstart operation cancelled.", - true) - if !yes { - return nil - } + return ruleTypes, ruleTypeNames, nil +} - cmd.Printf("Creating profile from remote catalog...\n") - profileReader, err := fs.Open(quickstartProfileFilePath) +func loadProfilesFromFS(fs billy.Filesystem) ([]*minderv1.Profile, error) { + paths, err := collectYAMLFiles(fs, catalogProfilesDir) if err != nil { - return fmt.Errorf("failed to open profile file: %w", err) + return nil, err + } + if len(paths) == 0 { + return nil, fmt.Errorf("no profile YAML files found under %s", catalogProfilesDir) } - defer profileReader.Close() - p, err := profiles.ParseYAML(profileReader) - if err != nil { - return fmt.Errorf("failed to parse profile: %w", err) + // Keep load order deterministic across filesystems. + sort.Strings(paths) + loadedProfiles := make([]*minderv1.Profile, 0, len(paths)) + profileNames := make(map[string]struct{}, len(paths)) + + for _, path := range paths { + if err := func() error { + reader, err := fs.Open(path) + if err != nil { + return fmt.Errorf("failed to open profile file %s: %w", path, err) + } + defer reader.Close() + + profile, err := profiles.ParseYAML(reader) + if err != nil { + return fmt.Errorf("failed to parse profile file %s: %w", path, err) + } + if profile.GetName() == "" { + return fmt.Errorf("profile file %s has no name", path) + } + if _, exists := profileNames[profile.GetName()]; exists { + return fmt.Errorf("duplicate profile %q found in %s", profile.GetName(), path) + } + profileNames[profile.GetName()] = struct{}{} + loadedProfiles = append(loadedProfiles, profile) + return nil + }(); err != nil { + return nil, err + } } - p.Context = &minderv1.Context{ - Provider: &provider, - Project: &project, + return loadedProfiles, nil +} + +func validateCatalog(ruleTypeNames map[string]struct{}, loadedProfiles []*minderv1.Profile) error { + for _, loadedProfile := range loadedProfiles { + referencedRuleTypes := make(map[string]struct{}) + if err := profiles.TraverseRuleTypesForEntities(loadedProfile, func(_ minderv1.Entity, rule *minderv1.Profile_Rule) error { + if rule.GetType() != "" { + referencedRuleTypes[rule.GetType()] = struct{}{} + } + return nil + }); err != nil { + return fmt.Errorf("failed to inspect profile %q: %w", loadedProfile.GetName(), err) + } + + for ruleType := range referencedRuleTypes { + if _, exists := ruleTypeNames[ruleType]; !exists { + return fmt.Errorf("profile %q references missing rule type %q", loadedProfile.GetName(), ruleType) + } + } } - ctx, cancel = getQuickstartContext(cmd.Context(), viper.GetViper()) - defer cancel() + return nil +} - alreadyExists := false - resp, err := profileClient.CreateProfile(ctx, &minderv1.CreateProfileRequest{Profile: p}) +func collectYAMLFiles(fs billy.Filesystem, root string) ([]string, error) { + entries, err := fs.ReadDir(root) if err != nil { - if st, ok := status.FromError(err); ok { - if st.Code() != codes.AlreadyExists { - return cli.MessageAndError("error creating profile", err) - } - alreadyExists = true - } else { - return cli.MessageAndError("error creating profile", err) + if os.IsNotExist(err) { + return nil, fmt.Errorf("catalog directory %s not found", root) } + return nil, fmt.Errorf("failed to read catalog directory %s: %w", root, err) } - if alreadyExists { - cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishExisting + stepPromptMsgFinishBase)) - } else { - cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishOK + stepPromptMsgFinishBase)) - cmd.Println("Profile details (minder profile list):") - table := profile.NewProfileRulesTable(cmd.OutOrStdout()) - profile.RenderProfileRulesTable(resp.GetProfile(), table) - table.Render() + paths := make([]string, 0, len(entries)) + for _, entry := range entries { + path := filepath.Join(root, entry.Name()) + if entry.IsDir() { + nested, err := collectYAMLFiles(fs, path) + if err != nil { + return nil, err + } + paths = append(paths, nested...) + continue + } + + switch strings.ToLower(filepath.Ext(entry.Name())) { + case ".yaml", ".yml": + paths = append(paths, path) + } } - return nil + return paths, nil } -// runExistingFlow contains the original quickstart catalog logic that uses -// the embedded secret_scanning rule type and profile YAML files. +// applyCatalog creates all rule types and profiles from the catalog via gRPC services. // -// This function is used as a safe fallback when loading the catalog from -// the configured repository fails. It recreates the previous Step 3 and -// Step 4 behavior by: -// - reading secret_scanning.yaml and profile.yaml from the embedded FS, -// - parsing them into the appropriate rule type and profile structures, -// - applying the current provider/project context, and -// - creating the resources via the corresponding gRPC services, including -// handling AlreadyExists responses and printing the final banners and -// profile details table. -func runExistingFlow( +// 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. +// 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, - provider string, + catalog *Catalog, project string, - registeredRepos []string, ) error { - cmd.Println("Creating rule type...") - reader, err := content.Open("embed/secret_scanning.yaml") - if err != nil { - return cli.MessageAndError("error opening rule type", err) - } + ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) + defer cancel() - rt := &minderv1.RuleType{} - if err := minderv1.ParseResource(reader, rt); err != nil { - return cli.MessageAndError("error parsing rule type", err) + projectContext := &minderv1.Context{ + Project: &project, + } + result := applyCatalogResult{ + createdRuleTypes: make([]string, 0, len(catalog.RuleTypes)), + createdProfiles: make([]string, 0, len(catalog.Profiles)), } - if rt.Context == nil { - rt.Context = &minderv1.Context{} + if err := createCatalogRuleTypes(ctx, cmd, ruleClient, catalog.RuleTypes, projectContext, &result); err != nil { + rollbackCatalogResources(ctx, ruleClient, profileClient, result) + return err } - rt.Context = &minderv1.Context{ - Provider: &provider, - Project: &project, + if err := createCatalogProfiles(ctx, cmd, profileClient, catalog.Profiles, projectContext, &result); err != nil { + rollbackCatalogResources(ctx, ruleClient, profileClient, result) + return err } - ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) - defer cancel() + printCatalogSummary(cmd, result) + return nil +} - _, 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) - } - } +type applyCatalogResult struct { + createdRuleTypes []string + createdProfiles []string + summaryProfile *minderv1.Profile + seenExistingProfile bool +} - yes := cli.PrintYesNoPrompt(cmd, - fmt.Sprintf(stepPromptMsgProfile, strings.Join(registeredRepos, "\n")), - "Proceed?", - "Quickstart operation cancelled.", - true) - if !yes { - return nil +func rollbackCatalogResources( + ctx context.Context, + ruleClient minderv1.RuleTypeServiceClient, + profileClient minderv1.ProfileServiceClient, + result applyCatalogResult, +) { + for i := len(result.createdProfiles) - 1; i >= 0; i-- { + _, _ = profileClient.DeleteProfile(ctx, &minderv1.DeleteProfileRequest{Id: result.createdProfiles[i]}) } - - cmd.Println("Creating profile...") - reader, err = content.Open("embed/profile.yaml") - if err != nil { - return cli.MessageAndError("error opening profile", err) + for i := len(result.createdRuleTypes) - 1; i >= 0; i-- { + _, _ = ruleClient.DeleteRuleType(ctx, &minderv1.DeleteRuleTypeRequest{Id: result.createdRuleTypes[i]}) } +} - p, err := profiles.ParseYAML(reader) - if err != nil { - return cli.MessageAndError("error parsing profile", err) - } +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 - if p.Context == nil { - p.Context = &minderv1.Context{} - } + 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 + } + if st, ok := status.FromError(err); ok { + return fmt.Errorf("error creating rule type %s: %s", ruleType.GetName(), st.Message()) + } + return cli.MessageAndError(fmt.Sprintf("error creating rule type %s", ruleType.GetName()), err) + } - p.Context = &minderv1.Context{ - Provider: &provider, - Project: &project, + createdName := ruleType.GetName() + if resp != nil && resp.GetRuleType() != nil && resp.GetRuleType().GetName() != "" { + createdName = resp.GetRuleType().GetName() + } + result.createdRuleTypes = append(result.createdRuleTypes, createdName) } - ctx, cancel = getQuickstartContext(cmd.Context(), viper.GetViper()) - defer cancel() + return nil +} - alreadyExists := false - 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 + result.summaryProfile = profileResource + + 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 { + result.seenExistingProfile = true + result.summaryProfile = profileResource + continue + } + if st, ok := status.FromError(err); ok { + return fmt.Errorf("error creating profile %s: %s", profileResource.GetName(), st.Message()) + } + return cli.MessageAndError(fmt.Sprintf("error creating profile %s", profileResource.GetName()), err) + } + + if resp != nil && resp.GetProfile() != nil { + result.summaryProfile = resp.GetProfile() + if result.summaryProfile.GetId() != "" { + result.createdProfiles = append(result.createdProfiles, result.summaryProfile.GetId()) + } else { + result.createdProfiles = append(result.createdProfiles, profileResource.GetName()) } - alreadyExists = true } else { - return cli.MessageAndError("error creating profile", err) + result.createdProfiles = append(result.createdProfiles, profileResource.GetName()) } } - if alreadyExists { + return nil +} + +func printCatalogSummary(cmd *cobra.Command, result applyCatalogResult) { + if result.seenExistingProfile { cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishExisting + stepPromptMsgFinishBase)) } else { cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishOK + stepPromptMsgFinishBase)) - cmd.Println("Profile details (minder profile list):") - table := profile.NewProfileRulesTable(cmd.OutOrStdout()) - profile.RenderProfileRulesTable(resp.GetProfile(), table) - table.Render() } - return nil + if result.summaryProfile == nil { + return + } + + cmd.Println("Profile details (minder profile list):") + table := profile.NewProfileRulesTable(cmd.OutOrStdout()) + profile.RenderProfileRulesTable(result.summaryProfile, table) + table.Render() } func init() { diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go deleted file mode 100644 index be398edfc4..0000000000 --- a/internal/cli/catalog.go +++ /dev/null @@ -1,33 +0,0 @@ -// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors -// SPDX-License-Identifier: Apache-2.0 - -// Package cli contains reusable CLI helper logic for catalog operations -package cli - -import ( - "fmt" - - "github.com/go-git/go-billy/v5" - git "github.com/go-git/go-git/v5" - "github.com/go-git/go-git/v5/storage/memory" -) - -// CloneRepoFilesystem clones the given Git repository URL into an in-memory -// storage and returns its filesystem. Callers can use the returned -// filesystem to open files and traverse directories without touching disk. -func CloneRepoFilesystem(repoURL string) (billy.Filesystem, error) { - repo, err := git.Clone(memory.NewStorage(), nil, &git.CloneOptions{ - URL: repoURL, - Depth: 1, - }) - if err != nil { - return nil, fmt.Errorf("failed to clone repository %s: %w", repoURL, err) - } - - worktree, err := repo.Worktree() - if err != nil { - return nil, fmt.Errorf("failed to get worktree for %s: %w", repoURL, err) - } - - return worktree.Filesystem, nil -} From e9718ac62ee929cae48dab762af214b96f60d5d9 Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Thu, 23 Apr 2026 13:44:56 +0530 Subject: [PATCH 5/9] refactor(cli): align quickstart catalog loading with review feedback --- cmd/cli/app/quickstart/quickstart.go | 228 ++---------------------- internal/cli/catalog.go | 251 +++++++++++++++++++++++++++ pkg/profiles/util.go | 12 ++ 3 files changed, 278 insertions(+), 213 deletions(-) create mode 100644 internal/cli/catalog.go diff --git a/cmd/cli/app/quickstart/quickstart.go b/cmd/cli/app/quickstart/quickstart.go index a071dbf1c6..f1bdc27493 100644 --- a/cmd/cli/app/quickstart/quickstart.go +++ b/cmd/cli/app/quickstart/quickstart.go @@ -9,11 +9,8 @@ import ( "context" "fmt" "os" - "path/filepath" - "sort" "strings" - "github.com/go-git/go-billy/v5" git "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/storage/memory" "github.com/spf13/cobra" @@ -24,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 ( @@ -271,7 +267,7 @@ func quickstartCommand( return fmt.Errorf("failed to load catalog repo: %w", err) } - catalog, err := LoadCatalogFromFS(worktree.Filesystem) + catalog, err := internalcli.LoadCatalogFromFS(worktree.Filesystem, cmd.Printf) if err != nil { return fmt.Errorf("failed to load catalog: %w", err) } @@ -303,176 +299,8 @@ func quickstartCommand( const ( defaultQuickstartCatalogRepoURL = "https://github.com/mindersec/minder-rules-and-profiles" - catalogRuleTypesDir = "rule-types" - catalogProfilesDir = "profiles" ) -// Catalog represents a loaded collection of rule types and profiles from a filesystem. -type Catalog struct { - RuleTypes []*minderv1.RuleType - Profiles []*minderv1.Profile -} - -// LoadCatalogFromFS loads and validates all YAML resources under the catalog directories. -// -// It recursively scans the rule-types and profiles directories, loads every YAML file, -// and verifies that each rule type referenced by each profile exists in the loaded set. -func LoadCatalogFromFS(fs billy.Filesystem) (*Catalog, error) { - ruleTypes, ruleTypeNames, err := loadRuleTypesFromFS(fs) - if err != nil { - return nil, err - } - - loadedProfiles, err := loadProfilesFromFS(fs) - if err != nil { - return nil, err - } - - if err := validateCatalog(ruleTypeNames, loadedProfiles); err != nil { - return nil, err - } - - return &Catalog{RuleTypes: ruleTypes, Profiles: loadedProfiles}, nil -} - -func loadRuleTypesFromFS(fs billy.Filesystem) ([]*minderv1.RuleType, map[string]struct{}, error) { - paths, err := collectYAMLFiles(fs, catalogRuleTypesDir) - if err != nil { - return nil, nil, err - } - if len(paths) == 0 { - return nil, nil, fmt.Errorf("no rule type YAML files found under %s", catalogRuleTypesDir) - } - - // Keep load order deterministic across filesystems. - sort.Strings(paths) - ruleTypes := make([]*minderv1.RuleType, 0, len(paths)) - ruleTypeNames := make(map[string]struct{}, len(paths)) - - for _, path := range paths { - if err := func() error { - reader, err := fs.Open(path) - if err != nil { - return fmt.Errorf("failed to open rule type file %s: %w", path, err) - } - defer reader.Close() - - ruleType := &minderv1.RuleType{} - if err := minderv1.ParseResource(reader, ruleType); err != nil { - return fmt.Errorf("failed to parse rule type file %s: %w", path, err) - } - if ruleType.GetName() == "" { - return fmt.Errorf("rule type file %s has no name", path) - } - if _, exists := ruleTypeNames[ruleType.GetName()]; exists { - return fmt.Errorf("duplicate rule type %q found in %s", ruleType.GetName(), path) - } - ruleTypeNames[ruleType.GetName()] = struct{}{} - ruleTypes = append(ruleTypes, ruleType) - return nil - }(); err != nil { - return nil, nil, err - } - } - - return ruleTypes, ruleTypeNames, nil -} - -func loadProfilesFromFS(fs billy.Filesystem) ([]*minderv1.Profile, error) { - paths, err := collectYAMLFiles(fs, catalogProfilesDir) - if err != nil { - return nil, err - } - if len(paths) == 0 { - return nil, fmt.Errorf("no profile YAML files found under %s", catalogProfilesDir) - } - - // Keep load order deterministic across filesystems. - sort.Strings(paths) - loadedProfiles := make([]*minderv1.Profile, 0, len(paths)) - profileNames := make(map[string]struct{}, len(paths)) - - for _, path := range paths { - if err := func() error { - reader, err := fs.Open(path) - if err != nil { - return fmt.Errorf("failed to open profile file %s: %w", path, err) - } - defer reader.Close() - - profile, err := profiles.ParseYAML(reader) - if err != nil { - return fmt.Errorf("failed to parse profile file %s: %w", path, err) - } - if profile.GetName() == "" { - return fmt.Errorf("profile file %s has no name", path) - } - if _, exists := profileNames[profile.GetName()]; exists { - return fmt.Errorf("duplicate profile %q found in %s", profile.GetName(), path) - } - profileNames[profile.GetName()] = struct{}{} - loadedProfiles = append(loadedProfiles, profile) - return nil - }(); err != nil { - return nil, err - } - } - - return loadedProfiles, nil -} - -func validateCatalog(ruleTypeNames map[string]struct{}, loadedProfiles []*minderv1.Profile) error { - for _, loadedProfile := range loadedProfiles { - referencedRuleTypes := make(map[string]struct{}) - if err := profiles.TraverseRuleTypesForEntities(loadedProfile, func(_ minderv1.Entity, rule *minderv1.Profile_Rule) error { - if rule.GetType() != "" { - referencedRuleTypes[rule.GetType()] = struct{}{} - } - return nil - }); err != nil { - return fmt.Errorf("failed to inspect profile %q: %w", loadedProfile.GetName(), err) - } - - for ruleType := range referencedRuleTypes { - if _, exists := ruleTypeNames[ruleType]; !exists { - return fmt.Errorf("profile %q references missing rule type %q", loadedProfile.GetName(), ruleType) - } - } - } - - return nil -} - -func collectYAMLFiles(fs billy.Filesystem, root string) ([]string, error) { - entries, err := fs.ReadDir(root) - if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("catalog directory %s not found", root) - } - return nil, fmt.Errorf("failed to read catalog directory %s: %w", root, err) - } - - paths := make([]string, 0, len(entries)) - for _, entry := range entries { - path := filepath.Join(root, entry.Name()) - if entry.IsDir() { - nested, err := collectYAMLFiles(fs, path) - if err != nil { - return nil, err - } - paths = append(paths, nested...) - continue - } - - switch strings.ToLower(filepath.Ext(entry.Name())) { - case ".yaml", ".yml": - paths = append(paths, path) - } - } - - return paths, nil -} - // 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 @@ -483,7 +311,7 @@ func applyCatalog( cmd *cobra.Command, ruleClient minderv1.RuleTypeServiceClient, profileClient minderv1.ProfileServiceClient, - catalog *Catalog, + catalog *internalcli.Catalog, project string, ) error { ctx, cancel := getQuickstartContext(cmd.Context(), viper.GetViper()) @@ -514,7 +342,6 @@ func applyCatalog( type applyCatalogResult struct { createdRuleTypes []string createdProfiles []string - summaryProfile *minderv1.Profile seenExistingProfile bool } @@ -524,11 +351,11 @@ func rollbackCatalogResources( profileClient minderv1.ProfileServiceClient, result applyCatalogResult, ) { - for i := len(result.createdProfiles) - 1; i >= 0; i-- { - _, _ = profileClient.DeleteProfile(ctx, &minderv1.DeleteProfileRequest{Id: result.createdProfiles[i]}) + for _, profileID := range result.createdProfiles { + _, _ = profileClient.DeleteProfile(ctx, &minderv1.DeleteProfileRequest{Id: profileID}) } - for i := len(result.createdRuleTypes) - 1; i >= 0; i-- { - _, _ = ruleClient.DeleteRuleType(ctx, &minderv1.DeleteRuleTypeRequest{Id: result.createdRuleTypes[i]}) + for _, ruleTypeID := range result.createdRuleTypes { + _, _ = ruleClient.DeleteRuleType(ctx, &minderv1.DeleteRuleTypeRequest{Id: ruleTypeID}) } } @@ -550,17 +377,14 @@ func createCatalogRuleTypes( cmd.Printf("Rule type %s already exists\n", ruleType.GetName()) continue } - if st, ok := status.FromError(err); ok { - return fmt.Errorf("error creating rule type %s: %s", ruleType.GetName(), st.Message()) - } - return cli.MessageAndError(fmt.Sprintf("error creating rule type %s", ruleType.GetName()), err) + return fmt.Errorf("error creating rule type %s: %w", ruleType.GetName(), err) } - createdName := ruleType.GetName() - if resp != nil && resp.GetRuleType() != nil && resp.GetRuleType().GetName() != "" { - createdName = resp.GetRuleType().GetName() + name := resp.GetRuleType().GetName() + if name == "" { + name = ruleType.GetName() } - result.createdRuleTypes = append(result.createdRuleTypes, createdName) + result.createdRuleTypes = append(result.createdRuleTypes, name) } return nil @@ -576,32 +400,19 @@ func createCatalogProfiles( ) error { for _, profileResource := range loadedProfiles { profileResource.Context = projectContext - result.summaryProfile = profileResource 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 - result.summaryProfile = profileResource continue } - if st, ok := status.FromError(err); ok { - return fmt.Errorf("error creating profile %s: %s", profileResource.GetName(), st.Message()) - } - return cli.MessageAndError(fmt.Sprintf("error creating profile %s", profileResource.GetName()), err) + return fmt.Errorf("error creating profile %s: %w", profileResource.GetName(), err) } - if resp != nil && resp.GetProfile() != nil { - result.summaryProfile = resp.GetProfile() - if result.summaryProfile.GetId() != "" { - result.createdProfiles = append(result.createdProfiles, result.summaryProfile.GetId()) - } else { - result.createdProfiles = append(result.createdProfiles, profileResource.GetName()) - } - } else { - result.createdProfiles = append(result.createdProfiles, profileResource.GetName()) - } + result.createdProfiles = append(result.createdProfiles, resp.GetProfile().GetId()) } return nil @@ -613,15 +424,6 @@ func printCatalogSummary(cmd *cobra.Command, result applyCatalogResult) { } else { cmd.Println(cli.WarningBanner.Render(stepPromptMsgFinishOK + stepPromptMsgFinishBase)) } - - if result.summaryProfile == nil { - return - } - - cmd.Println("Profile details (minder profile list):") - table := profile.NewProfileRulesTable(cmd.OutOrStdout()) - profile.RenderProfileRulesTable(result.summaryProfile, table) - table.Render() } func init() { diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go new file mode 100644 index 0000000000..9fc0fcdd00 --- /dev/null +++ b/internal/cli/catalog.go @@ -0,0 +1,251 @@ +// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + +// Package cli provides internal CLI helpers shared by command implementations. +package cli + +import ( + "fmt" + "io" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + + "github.com/go-git/go-billy/v5" + billyutil "github.com/go-git/go-billy/v5/util" + + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/fileconvert" + "github.com/mindersec/minder/pkg/profiles" +) + +const ( + catalogRuleTypesDir = "rule-types" + catalogProfilesDir = "profiles" +) + +// Catalog represents a loaded collection of rule types and profiles from a filesystem. +type Catalog struct { + RuleTypes []*minderv1.RuleType + Profiles []*minderv1.Profile +} + +// LoadCatalogFromFS loads and validates all resources under the catalog directories. +// Invalid resources are skipped and reported through warnf. +func LoadCatalogFromFS(vfs billy.Filesystem, warnf func(string, ...any)) (*Catalog, error) { + if warnf == nil { + warnf = func(string, ...any) {} + } + + ruleTypesByName, err := loadRuleTypesFromFS(vfs, warnf) + if err != nil { + return nil, err + } + + loadedProfiles, err := loadProfilesFromFS(vfs, warnf) + if err != nil { + return nil, err + } + + validProfiles := validateCatalog(ruleTypesByName, loadedProfiles, warnf) + if len(validProfiles) == 0 { + return nil, fmt.Errorf("no valid profiles found under %s", catalogProfilesDir) + } + + ruleTypeNames := make([]string, 0, len(ruleTypesByName)) + for name := range ruleTypesByName { + ruleTypeNames = append(ruleTypeNames, name) + } + sort.Strings(ruleTypeNames) + + ruleTypes := make([]*minderv1.RuleType, 0, len(ruleTypeNames)) + for _, name := range ruleTypeNames { + ruleTypes = append(ruleTypes, ruleTypesByName[name]) + } + + return &Catalog{RuleTypes: ruleTypes, Profiles: validProfiles}, nil +} + +func loadRuleTypesFromFS(vfs billy.Filesystem, warnf func(string, ...any)) (map[string]*minderv1.RuleType, error) { + paths, err := collectYAMLFiles(vfs, catalogRuleTypesDir) + if err != nil { + return nil, err + } + if len(paths) == 0 { + return nil, fmt.Errorf("no rule type YAML files found under %s", catalogRuleTypesDir) + } + + ruleTypesByName := make(map[string]*minderv1.RuleType, len(paths)) + for _, path := range paths { + ruleType, err := readRuleTypeFromPath(vfs, path) + if err != nil { + warnf("Skipping invalid rule type %s: %v\n", path, err) + continue + } + if ruleType.GetName() == "" { + warnf("Skipping invalid rule type %s: missing name\n", path) + continue + } + if _, exists := ruleTypesByName[ruleType.GetName()]; exists { + warnf("Skipping duplicate rule type %q from %s\n", ruleType.GetName(), path) + continue + } + ruleTypesByName[ruleType.GetName()] = ruleType + } + + if len(ruleTypesByName) == 0 { + return nil, fmt.Errorf("no valid rule types found under %s", catalogRuleTypesDir) + } + + return ruleTypesByName, nil +} + +func loadProfilesFromFS(vfs billy.Filesystem, warnf func(string, ...any)) ([]*minderv1.Profile, error) { + paths, err := collectYAMLFiles(vfs, catalogProfilesDir) + if err != nil { + return nil, err + } + if len(paths) == 0 { + return nil, fmt.Errorf("no profile YAML files found under %s", catalogProfilesDir) + } + + loadedProfiles := make([]*minderv1.Profile, 0, len(paths)) + profileNames := make(map[string]struct{}, len(paths)) + + for _, path := range paths { + profile, err := profiles.ReadProfileFromPath(vfs, path) + if err != nil { + warnf("Skipping invalid profile %s: %v\n", path, err) + continue + } + if profile.GetName() == "" { + warnf("Skipping invalid profile %s: missing name\n", path) + continue + } + if _, exists := profileNames[profile.GetName()]; exists { + warnf("Skipping duplicate profile %q from %s\n", profile.GetName(), path) + continue + } + + profileNames[profile.GetName()] = struct{}{} + loadedProfiles = append(loadedProfiles, profile) + } + + if len(loadedProfiles) == 0 { + return nil, fmt.Errorf("no valid profiles found under %s", catalogProfilesDir) + } + + return loadedProfiles, nil +} + +func validateCatalog( + ruleTypesByName map[string]*minderv1.RuleType, + loadedProfiles []*minderv1.Profile, + warnf func(string, ...any), +) []*minderv1.Profile { + validProfiles := make([]*minderv1.Profile, 0, len(loadedProfiles)) + + for _, loadedProfile := range loadedProfiles { + referencedRuleTypes := make(map[string]struct{}) + if err := profiles.TraverseRuleTypesForEntities(loadedProfile, func(_ minderv1.Entity, rule *minderv1.Profile_Rule) error { + if rule.GetType() != "" { + referencedRuleTypes[rule.GetType()] = struct{}{} + } + return nil + }); err != nil { + warnf("Skipping invalid profile %q: failed to inspect rules: %v\n", loadedProfile.GetName(), err) + continue + } + + missingRuleTypes := make([]string, 0) + for ruleType := range referencedRuleTypes { + if _, exists := ruleTypesByName[ruleType]; !exists { + missingRuleTypes = append(missingRuleTypes, ruleType) + } + } + + if len(missingRuleTypes) > 0 { + sort.Strings(missingRuleTypes) + warnf( + "Skipping profile %q: references missing rule type(s): %s\n", + loadedProfile.GetName(), + strings.Join(missingRuleTypes, ", "), + ) + continue + } + + validProfiles = append(validProfiles, loadedProfile) + } + + return validProfiles +} + +func collectYAMLFiles(vfs billy.Filesystem, root string) ([]string, error) { + paths := make([]string, 0) + err := billyutil.Walk(vfs, root, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + switch strings.ToLower(filepath.Ext(path)) { + case ".yaml", ".yml": + paths = append(paths, path) + } + return nil + }) + if err != nil { + if os.IsNotExist(err) { + return nil, fmt.Errorf("catalog directory %s not found", root) + } + return nil, fmt.Errorf("failed to read catalog directory %s: %w", root, err) + } + if len(paths) == 0 { + return nil, nil + } + sort.Strings(paths) + return paths, nil +} + +func readRuleTypeFromPath(vfs billy.Filesystem, path string) (*minderv1.RuleType, error) { + reader, err := vfs.Open(path) + if err != nil { + return nil, fmt.Errorf("failed to open rule type file %s: %w", path, err) + } + defer reader.Close() + + ext := filepath.Ext(path) + if ext == "" { + ext = ".yaml" + } + tmpFile, err := os.CreateTemp("", "minder-quickstart-ruletype-*"+ext) + if err != nil { + return nil, fmt.Errorf("failed to create temp file for %s: %w", path, err) + } + tmpName := tmpFile.Name() + defer os.Remove(tmpName) + + if _, err := io.Copy(tmpFile, reader); err != nil { + _ = tmpFile.Close() + return nil, fmt.Errorf("failed to copy rule type file %s: %w", path, err) + } + if err := tmpFile.Close(); err != nil { + return nil, fmt.Errorf("failed to close temp file for %s: %w", path, err) + } + + decoder, closer := fileconvert.DecoderForFile(tmpName) + if decoder == nil { + return nil, fmt.Errorf("unsupported rule type format for %s", path) + } + defer closer.Close() + + ruleType, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) + if err != nil { + return nil, fmt.Errorf("failed to parse rule type file %s: %w", path, err) + } + + return ruleType, nil +} diff --git a/pkg/profiles/util.go b/pkg/profiles/util.go index 4a3fcbfff4..b83cc2f41b 100644 --- a/pkg/profiles/util.go +++ b/pkg/profiles/util.go @@ -11,6 +11,7 @@ import ( "regexp" "strings" + "github.com/go-git/go-billy/v5" "github.com/rs/zerolog/log" "github.com/sqlc-dev/pqtype" "google.golang.org/protobuf/proto" @@ -82,6 +83,17 @@ func ReadProfileFromFile(fpath string) (*pb.Profile, error) { return fileconvert.ReadResourceTyped[*pb.Profile](decoder) } +// ReadProfileFromPath reads a pipeline profile from a billy filesystem path. +func ReadProfileFromPath(vfs billy.Filesystem, path string) (*pb.Profile, error) { + reader, err := vfs.Open(path) + if err != nil { + return nil, fmt.Errorf("error opening profile path %s: %w", path, err) + } + defer reader.Close() + + return ParseYAML(reader) +} + // GetRulesForEntity returns the rules for the given entity func GetRulesForEntity(p *pb.Profile, entity pb.Entity) ([]*pb.Profile_Rule, error) { switch entity { From 0891c99a73aa91be0136d6d15df00f0e8f3b2648 Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Thu, 23 Apr 2026 22:45:11 +0530 Subject: [PATCH 6/9] refactor(cli): reuse fileconvert resources loader and align catalog loading --- cmd/cli/app/profile/common.go | 5 +- internal/cli/catalog.go | 199 +++++++++------------------- pkg/fileconvert/collect.go | 48 +++++++ pkg/fileconvert/encodedecode.go | 47 +++++-- pkg/fileconvert/fileconvert_test.go | 22 +++ pkg/mindpak/reader/reader.go | 5 +- pkg/profiles/util.go | 8 +- 7 files changed, 176 insertions(+), 158 deletions(-) diff --git a/cmd/cli/app/profile/common.go b/cmd/cli/app/profile/common.go index f8b74cd147..85eb770e32 100644 --- a/cmd/cli/app/profile/common.go +++ b/cmd/cli/app/profile/common.go @@ -8,13 +8,14 @@ import ( "fmt" "io" + "github.com/mindersec/minder/pkg/fileconvert" "github.com/spf13/viper" + "gopkg.in/yaml.v3" "github.com/mindersec/minder/internal/util" "github.com/mindersec/minder/internal/util/cli" "github.com/mindersec/minder/internal/util/cli/table" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" - "github.com/mindersec/minder/pkg/profiles" ) // ExecOnOneProfile is a helper function to execute a function on a single profile @@ -46,7 +47,7 @@ func ExecOnOneProfile(ctx context.Context, t table.Table, f string, dashOpen io. } func parseProfile(r io.Reader, proj string) (*minderv1.Profile, error) { - p, err := profiles.ParseYAML(r) + p, err := fileconvert.ReadResourceTyped[*minderv1.Profile](yaml.NewDecoder(r)) if err != nil { return nil, fmt.Errorf("error reading profile from file: %w", err) } diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go index 9fc0fcdd00..1fc10b812a 100644 --- a/internal/cli/catalog.go +++ b/internal/cli/catalog.go @@ -6,15 +6,10 @@ package cli import ( "fmt" - "io" - "io/fs" - "os" - "path/filepath" "sort" "strings" "github.com/go-git/go-billy/v5" - billyutil "github.com/go-git/go-billy/v5/util" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/fileconvert" @@ -32,9 +27,12 @@ type Catalog struct { Profiles []*minderv1.Profile } +// WarnFunc reports skipped catalog resources. +type WarnFunc = fileconvert.Printer + // LoadCatalogFromFS loads and validates all resources under the catalog directories. // Invalid resources are skipped and reported through warnf. -func LoadCatalogFromFS(vfs billy.Filesystem, warnf func(string, ...any)) (*Catalog, error) { +func LoadCatalogFromFS(vfs billy.Filesystem, warnf WarnFunc) (*Catalog, error) { if warnf == nil { warnf = func(string, ...any) {} } @@ -49,11 +47,6 @@ func LoadCatalogFromFS(vfs billy.Filesystem, warnf func(string, ...any)) (*Catal return nil, err } - validProfiles := validateCatalog(ruleTypesByName, loadedProfiles, warnf) - if len(validProfiles) == 0 { - return nil, fmt.Errorf("no valid profiles found under %s", catalogProfilesDir) - } - ruleTypeNames := make([]string, 0, len(ruleTypesByName)) for name := range ruleTypesByName { ruleTypeNames = append(ruleTypeNames, name) @@ -65,89 +58,32 @@ func LoadCatalogFromFS(vfs billy.Filesystem, warnf func(string, ...any)) (*Catal ruleTypes = append(ruleTypes, ruleTypesByName[name]) } - return &Catalog{RuleTypes: ruleTypes, Profiles: validProfiles}, nil -} - -func loadRuleTypesFromFS(vfs billy.Filesystem, warnf func(string, ...any)) (map[string]*minderv1.RuleType, error) { - paths, err := collectYAMLFiles(vfs, catalogRuleTypesDir) - if err != nil { + catalog := &Catalog{RuleTypes: ruleTypes, Profiles: loadedProfiles} + if err := catalog.Validate(warnf); err != nil { return nil, err } - if len(paths) == 0 { - return nil, fmt.Errorf("no rule type YAML files found under %s", catalogRuleTypesDir) - } - - ruleTypesByName := make(map[string]*minderv1.RuleType, len(paths)) - for _, path := range paths { - ruleType, err := readRuleTypeFromPath(vfs, path) - if err != nil { - warnf("Skipping invalid rule type %s: %v\n", path, err) - continue - } - if ruleType.GetName() == "" { - warnf("Skipping invalid rule type %s: missing name\n", path) - continue - } - if _, exists := ruleTypesByName[ruleType.GetName()]; exists { - warnf("Skipping duplicate rule type %q from %s\n", ruleType.GetName(), path) - continue - } - ruleTypesByName[ruleType.GetName()] = ruleType - } - - if len(ruleTypesByName) == 0 { - return nil, fmt.Errorf("no valid rule types found under %s", catalogRuleTypesDir) - } - return ruleTypesByName, nil + return catalog, nil } -func loadProfilesFromFS(vfs billy.Filesystem, warnf func(string, ...any)) ([]*minderv1.Profile, error) { - paths, err := collectYAMLFiles(vfs, catalogProfilesDir) - if err != nil { - return nil, err +// Validate validates the catalog contents and keeps only profiles whose rule types exist. +func (c *Catalog) Validate(warnf WarnFunc) error { + if c == nil { + return fmt.Errorf("catalog is nil") } - if len(paths) == 0 { - return nil, fmt.Errorf("no profile YAML files found under %s", catalogProfilesDir) + if warnf == nil { + warnf = func(string, ...any) {} } - loadedProfiles := make([]*minderv1.Profile, 0, len(paths)) - profileNames := make(map[string]struct{}, len(paths)) - - for _, path := range paths { - profile, err := profiles.ReadProfileFromPath(vfs, path) - if err != nil { - warnf("Skipping invalid profile %s: %v\n", path, err) - continue - } - if profile.GetName() == "" { - warnf("Skipping invalid profile %s: missing name\n", path) - continue - } - if _, exists := profileNames[profile.GetName()]; exists { - warnf("Skipping duplicate profile %q from %s\n", profile.GetName(), path) - continue + ruleTypesByName := make(map[string]struct{}, len(c.RuleTypes)) + for _, ruleType := range c.RuleTypes { + if name := ruleType.GetName(); name != "" { + ruleTypesByName[name] = struct{}{} } - - profileNames[profile.GetName()] = struct{}{} - loadedProfiles = append(loadedProfiles, profile) - } - - if len(loadedProfiles) == 0 { - return nil, fmt.Errorf("no valid profiles found under %s", catalogProfilesDir) } - return loadedProfiles, nil -} - -func validateCatalog( - ruleTypesByName map[string]*minderv1.RuleType, - loadedProfiles []*minderv1.Profile, - warnf func(string, ...any), -) []*minderv1.Profile { - validProfiles := make([]*minderv1.Profile, 0, len(loadedProfiles)) - - for _, loadedProfile := range loadedProfiles { + validProfiles := make([]*minderv1.Profile, 0, len(c.Profiles)) + for _, loadedProfile := range c.Profiles { referencedRuleTypes := make(map[string]struct{}) if err := profiles.TraverseRuleTypesForEntities(loadedProfile, func(_ minderv1.Entity, rule *minderv1.Profile_Rule) error { if rule.GetType() != "" { @@ -179,73 +115,66 @@ func validateCatalog( validProfiles = append(validProfiles, loadedProfile) } - return validProfiles + if len(validProfiles) == 0 { + return fmt.Errorf("no valid profiles found under %s", catalogProfilesDir) + } + + c.Profiles = validProfiles + return nil } -func collectYAMLFiles(vfs billy.Filesystem, root string) ([]string, error) { - paths := make([]string, 0) - err := billyutil.Walk(vfs, root, func(path string, info fs.FileInfo, err error) error { - if err != nil { - return err - } - if info.IsDir() { - return nil - } - switch strings.ToLower(filepath.Ext(path)) { - case ".yaml", ".yml": - paths = append(paths, path) - } - return nil - }) +func loadRuleTypesFromFS(vfs billy.Filesystem, warnf WarnFunc) (map[string]*minderv1.RuleType, error) { + ruleTypes, err := fileconvert.ResourcesFromFilesystem[*minderv1.RuleType](warnf, vfs, catalogRuleTypesDir) if err != nil { - if os.IsNotExist(err) { - return nil, fmt.Errorf("catalog directory %s not found", root) + return nil, err + } + + ruleTypesByName := make(map[string]*minderv1.RuleType, len(ruleTypes)) + for _, ruleType := range ruleTypes { + if ruleType.GetName() == "" { + warnf("Skipping invalid rule type: missing name\n") + continue } - return nil, fmt.Errorf("failed to read catalog directory %s: %w", root, err) + if _, exists := ruleTypesByName[ruleType.GetName()]; exists { + warnf("Skipping duplicate rule type %q\n", ruleType.GetName()) + continue + } + ruleTypesByName[ruleType.GetName()] = ruleType } - if len(paths) == 0 { - return nil, nil + + if len(ruleTypesByName) == 0 { + return nil, fmt.Errorf("no valid rule types found under %s", catalogRuleTypesDir) } - sort.Strings(paths) - return paths, nil + + return ruleTypesByName, nil } -func readRuleTypeFromPath(vfs billy.Filesystem, path string) (*minderv1.RuleType, error) { - reader, err := vfs.Open(path) +func loadProfilesFromFS(vfs billy.Filesystem, warnf WarnFunc) ([]*minderv1.Profile, error) { + profiles, err := fileconvert.ResourcesFromFilesystem[*minderv1.Profile](warnf, vfs, catalogProfilesDir) if err != nil { - return nil, fmt.Errorf("failed to open rule type file %s: %w", path, err) + return nil, err } - defer reader.Close() - ext := filepath.Ext(path) - if ext == "" { - ext = ".yaml" - } - tmpFile, err := os.CreateTemp("", "minder-quickstart-ruletype-*"+ext) - if err != nil { - return nil, fmt.Errorf("failed to create temp file for %s: %w", path, err) - } - tmpName := tmpFile.Name() - defer os.Remove(tmpName) + loadedProfiles := make([]*minderv1.Profile, 0, len(profiles)) + profileNames := make(map[string]struct{}, len(profiles)) - if _, err := io.Copy(tmpFile, reader); err != nil { - _ = tmpFile.Close() - return nil, fmt.Errorf("failed to copy rule type file %s: %w", path, err) - } - if err := tmpFile.Close(); err != nil { - return nil, fmt.Errorf("failed to close temp file for %s: %w", path, err) - } + for _, profile := range profiles { + if profile.GetName() == "" { + warnf("Skipping invalid profile: missing name\n") + continue + } + if _, exists := profileNames[profile.GetName()]; exists { + warnf("Skipping duplicate profile %q\n", profile.GetName()) + continue + } - decoder, closer := fileconvert.DecoderForFile(tmpName) - if decoder == nil { - return nil, fmt.Errorf("unsupported rule type format for %s", path) + profileNames[profile.GetName()] = struct{}{} + loadedProfiles = append(loadedProfiles, profile) } - defer closer.Close() - ruleType, err := fileconvert.ReadResourceTyped[*minderv1.RuleType](decoder) - if err != nil { - return nil, fmt.Errorf("failed to parse rule type file %s: %w", path, err) + if len(loadedProfiles) == 0 { + return nil, fmt.Errorf("no valid profiles found under %s", catalogProfilesDir) } - return ruleType, nil + return loadedProfiles, nil } diff --git a/pkg/fileconvert/collect.go b/pkg/fileconvert/collect.go index d113da487b..9f53a6615d 100644 --- a/pkg/fileconvert/collect.go +++ b/pkg/fileconvert/collect.go @@ -9,8 +9,15 @@ import ( "errors" "fmt" "io" + iofs "io/fs" "os" + "path/filepath" + "sort" + "strings" + "github.com/go-git/go-billy/v5" + billyutil "github.com/go-git/go-billy/v5/util" + "google.golang.org/protobuf/proto" "gopkg.in/yaml.v3" "github.com/mindersec/minder/internal/util" @@ -20,6 +27,47 @@ import ( // Printer provides an interface for passing a printf-like function. type Printer func(string, ...any) +// ResourcesFromFilesystem collects resources from a directory in a billy filesystem. +func ResourcesFromFilesystem[T proto.Message](printer Printer, fs billy.Filesystem, root string) ([]T, error) { + if printer == nil { + printer = func(string, ...any) {} + } + + paths := make([]string, 0) + err := billyutil.Walk(fs, root, func(path string, info iofs.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + switch strings.ToLower(filepath.Ext(path)) { + case ".yaml", ".yml", ".json": + paths = append(paths, path) + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("failed to read directory %s: %w", root, err) + } + if len(paths) == 0 { + return nil, nil + } + sort.Strings(paths) + + objects := make([]T, 0, len(paths)) + for _, path := range paths { + resource, err := ReadResourceFromFile[T](fs, path) + if err != nil { + printer("Skipping invalid file %s: %v\n", path, err) + continue + } + objects = append(objects, resource) + } + + return objects, nil +} + // ResourcesFromPaths collects func ResourcesFromPaths(printer Printer, paths ...string) ([]minderv1.ResourceMeta, error) { files, err := util.ExpandFileArgs(paths...) diff --git a/pkg/fileconvert/encodedecode.go b/pkg/fileconvert/encodedecode.go index 6aee488030..c379699ab1 100644 --- a/pkg/fileconvert/encodedecode.go +++ b/pkg/fileconvert/encodedecode.go @@ -12,6 +12,7 @@ import ( "os" "path/filepath" + "github.com/go-git/go-billy/v5" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" "gopkg.in/yaml.v3" @@ -32,26 +33,48 @@ type Decoder interface { var _ Decoder = (*yaml.Decoder)(nil) var _ Decoder = (*json.Decoder)(nil) -// DecoderForFile returns a Decoder for the file at the specified path, -// or nil if the file is not of the appropriate type. -func DecoderForFile(path string) (Decoder, io.Closer) { - path = filepath.Clean(path) - ext := filepath.Ext(path) - var builder func(io.Reader) Decoder - // we return functions here so that we can early-exit without opening the file if the extension is unmatched. - switch ext { +func decoderForReader(path string, reader io.Reader) (Decoder, error) { + switch filepath.Ext(filepath.Clean(path)) { case ".json": - builder = func(r io.Reader) Decoder { return json.NewDecoder(r) } + return json.NewDecoder(reader), nil case ".yaml", ".yml": - builder = func(r io.Reader) Decoder { return yaml.NewDecoder(r) } + return yaml.NewDecoder(reader), nil default: - return nil, nil + return nil, fmt.Errorf("unsupported file format: %s", path) } +} + +// DecoderForFile returns a Decoder for the file at the specified path, +// or nil if the file is not of the appropriate type. +func DecoderForFile(path string) (Decoder, io.Closer) { file, err := os.Open(path) if err != nil { return nil, nil } - return builder(file), file + decoder, decoderErr := decoderForReader(path, file) + if decoderErr != nil { + _ = file.Close() + return nil, nil + } + return decoder, file +} + +// ReadResourceFromFile reads a single resource from the specified filesystem path. +func ReadResourceFromFile[T proto.Message](fs billy.Filesystem, path string) (T, error) { + var zero T + + file, err := fs.Open(path) + if err != nil { + return zero, fmt.Errorf("error opening file %s: %w", path, err) + } + defer file.Close() + + decoder, decoderErr := decoderForReader(path, file) + if decoderErr != nil { + return zero, decoderErr + } + + return ReadResourceTyped[T](decoder) } // WriteResource outputs a Minder proto resource to an existing Encoder. diff --git a/pkg/fileconvert/fileconvert_test.go b/pkg/fileconvert/fileconvert_test.go index 761844aaf1..820857d77a 100644 --- a/pkg/fileconvert/fileconvert_test.go +++ b/pkg/fileconvert/fileconvert_test.go @@ -14,6 +14,7 @@ import ( "strings" "testing" + "github.com/go-git/go-billy/v5/memfs" gocmp "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -193,6 +194,27 @@ func TestReadResourceTyped(t *testing.T) { require.NoError(t, err, "Expected no error reading profile") } +func TestReadResourceFromFile(t *testing.T) { + t.Parallel() + + fs := memfs.New() + file, err := fs.Create("profile.yaml") + require.NoError(t, err) + _, err = file.Write([]byte(`type: profile +version: v1 +name: test-profile +repository: + - type: sample-rule + def: {} +`)) + require.NoError(t, err) + require.NoError(t, file.Close()) + + profile, err := ReadResourceFromFile[*minderv1.Profile](fs, "profile.yaml") + require.NoError(t, err) + require.Equal(t, "test-profile", profile.GetName()) +} + func TestReadAll(t *testing.T) { t.Parallel() diff --git a/pkg/mindpak/reader/reader.go b/pkg/mindpak/reader/reader.go index aaba15eae5..4449d9d425 100644 --- a/pkg/mindpak/reader/reader.go +++ b/pkg/mindpak/reader/reader.go @@ -10,8 +10,9 @@ import ( "strings" v1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/fileconvert" "github.com/mindersec/minder/pkg/mindpak" - "github.com/mindersec/minder/pkg/profiles" + "gopkg.in/yaml.v3" ) // BundleReader provides a high-level interface for accessing the contents of @@ -82,7 +83,7 @@ func (b *bundleReader) GetProfile(name string) (*v1.Profile, error) { defer file.Close() // parse profile from YAML - profile, err := profiles.ParseYAML(file) + profile, err := fileconvert.ReadResourceTyped[*v1.Profile](yaml.NewDecoder(file)) if err != nil { return nil, fmt.Errorf("error parsing profile yaml: %w", err) } diff --git a/pkg/profiles/util.go b/pkg/profiles/util.go index b83cc2f41b..9e4f4c92d4 100644 --- a/pkg/profiles/util.go +++ b/pkg/profiles/util.go @@ -85,13 +85,7 @@ func ReadProfileFromFile(fpath string) (*pb.Profile, error) { // ReadProfileFromPath reads a pipeline profile from a billy filesystem path. func ReadProfileFromPath(vfs billy.Filesystem, path string) (*pb.Profile, error) { - reader, err := vfs.Open(path) - if err != nil { - return nil, fmt.Errorf("error opening profile path %s: %w", path, err) - } - defer reader.Close() - - return ParseYAML(reader) + return fileconvert.ReadResourceFromFile[*pb.Profile](vfs, path) } // GetRulesForEntity returns the rules for the given entity From ece5e51c08aa7a907ec9ffb77869241e07d097a3 Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Thu, 23 Apr 2026 22:47:53 +0530 Subject: [PATCH 7/9] chore(cli): fix lint issues and polish catalog loading implementation --- cmd/cli/app/profile/common.go | 2 +- internal/cli/catalog.go | 8 ++++---- pkg/fileconvert/encodedecode.go | 2 +- pkg/mindpak/reader/reader.go | 3 ++- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/cmd/cli/app/profile/common.go b/cmd/cli/app/profile/common.go index 85eb770e32..e7b6c40e30 100644 --- a/cmd/cli/app/profile/common.go +++ b/cmd/cli/app/profile/common.go @@ -8,7 +8,6 @@ import ( "fmt" "io" - "github.com/mindersec/minder/pkg/fileconvert" "github.com/spf13/viper" "gopkg.in/yaml.v3" @@ -16,6 +15,7 @@ import ( "github.com/mindersec/minder/internal/util/cli" "github.com/mindersec/minder/internal/util/cli/table" minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" + "github.com/mindersec/minder/pkg/fileconvert" ) // ExecOnOneProfile is a helper function to execute a function on a single profile diff --git a/internal/cli/catalog.go b/internal/cli/catalog.go index 1fc10b812a..80e8856e2b 100644 --- a/internal/cli/catalog.go +++ b/internal/cli/catalog.go @@ -150,15 +150,15 @@ func loadRuleTypesFromFS(vfs billy.Filesystem, warnf WarnFunc) (map[string]*mind } func loadProfilesFromFS(vfs billy.Filesystem, warnf WarnFunc) ([]*minderv1.Profile, error) { - profiles, err := fileconvert.ResourcesFromFilesystem[*minderv1.Profile](warnf, vfs, catalogProfilesDir) + loadedProfilesFromFS, err := fileconvert.ResourcesFromFilesystem[*minderv1.Profile](warnf, vfs, catalogProfilesDir) if err != nil { return nil, err } - loadedProfiles := make([]*minderv1.Profile, 0, len(profiles)) - profileNames := make(map[string]struct{}, len(profiles)) + loadedProfiles := make([]*minderv1.Profile, 0, len(loadedProfilesFromFS)) + profileNames := make(map[string]struct{}, len(loadedProfilesFromFS)) - for _, profile := range profiles { + for _, profile := range loadedProfilesFromFS { if profile.GetName() == "" { warnf("Skipping invalid profile: missing name\n") continue diff --git a/pkg/fileconvert/encodedecode.go b/pkg/fileconvert/encodedecode.go index c379699ab1..733eecf990 100644 --- a/pkg/fileconvert/encodedecode.go +++ b/pkg/fileconvert/encodedecode.go @@ -47,7 +47,7 @@ func decoderForReader(path string, reader io.Reader) (Decoder, error) { // DecoderForFile returns a Decoder for the file at the specified path, // or nil if the file is not of the appropriate type. func DecoderForFile(path string) (Decoder, io.Closer) { - file, err := os.Open(path) + file, err := os.Open(path) // #nosec G304 -- path is limited to on-disk catalog inputs if err != nil { return nil, nil } diff --git a/pkg/mindpak/reader/reader.go b/pkg/mindpak/reader/reader.go index 4449d9d425..7be4bdcdfe 100644 --- a/pkg/mindpak/reader/reader.go +++ b/pkg/mindpak/reader/reader.go @@ -9,10 +9,11 @@ import ( "io/fs" "strings" + "gopkg.in/yaml.v3" + v1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" "github.com/mindersec/minder/pkg/fileconvert" "github.com/mindersec/minder/pkg/mindpak" - "gopkg.in/yaml.v3" ) // BundleReader provides a high-level interface for accessing the contents of From a061c153c3e49a0766d3564f3d330f34953b1a35 Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Fri, 24 Apr 2026 00:30:55 +0530 Subject: [PATCH 8/9] test(cli): add coverage for catalog validation edge cases --- internal/cli/catalog_test.go | 89 ++++++++++++++++++++++++++++++++++++ 1 file changed, 89 insertions(+) create mode 100644 internal/cli/catalog_test.go diff --git a/internal/cli/catalog_test.go b/internal/cli/catalog_test.go new file mode 100644 index 0000000000..7b1afb8f3d --- /dev/null +++ b/internal/cli/catalog_test.go @@ -0,0 +1,89 @@ +package cli + +import ( + "testing" + + "github.com/stretchr/testify/require" + + minderv1 "github.com/mindersec/minder/pkg/api/protobuf/go/minder/v1" +) + +func TestCatalogValidate_NilCatalog(t *testing.T) { + t.Parallel() + var catalog *Catalog + + require.NotPanics(t, func() { + err := catalog.Validate(nil) + require.Error(t, err) + }) +} + +func TestCatalogValidate_EmptyCatalog(t *testing.T) { + t.Parallel() + catalog := &Catalog{ + RuleTypes: []*minderv1.RuleType{}, + Profiles: []*minderv1.Profile{}, + } + + err := catalog.Validate(nil) + require.Error(t, err) +} + +func TestCatalogValidate_WithValidRuleReference(t *testing.T) { + t.Parallel() + ruleType := &minderv1.RuleType{ + Name: "test-rule", + } + + profile := &minderv1.Profile{ + Name: "test-profile", + Repository: []*minderv1.Profile_Rule{ + { + Type: "test-rule", + }, + }, + } + + catalog := &Catalog{ + RuleTypes: []*minderv1.RuleType{ruleType}, + Profiles: []*minderv1.Profile{profile}, + } + + err := catalog.Validate(func(string, ...any) {}) + require.NoError(t, err) +} + +func TestCatalogValidate_SkipsProfilesWithMissingRuleTypes(t *testing.T) { + t.Parallel() + ruleType := &minderv1.RuleType{ + Name: "test-rule", + } + + validProfile := &minderv1.Profile{ + Name: "valid-profile", + Repository: []*minderv1.Profile_Rule{ + { + Type: "test-rule", + }, + }, + } + + invalidProfile := &minderv1.Profile{ + Name: "invalid-profile", + Repository: []*minderv1.Profile_Rule{ + { + Type: "missing-rule", + }, + }, + } + + catalog := &Catalog{ + RuleTypes: []*minderv1.RuleType{ruleType}, + Profiles: []*minderv1.Profile{validProfile, invalidProfile}, + } + + err := catalog.Validate(func(string, ...any) {}) + require.NoError(t, err) + require.Len(t, catalog.Profiles, 1) + require.Equal(t, "valid-profile", catalog.Profiles[0].Name) +} From e726e9002fedf022bf1eff099e77d1e5d105429c Mon Sep 17 00:00:00 2001 From: Sachin Kumar Date: Fri, 24 Apr 2026 00:35:03 +0530 Subject: [PATCH 9/9] Add SPDX header to catalog tests --- internal/cli/catalog_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/cli/catalog_test.go b/internal/cli/catalog_test.go index 7b1afb8f3d..d1b2f2340c 100644 --- a/internal/cli/catalog_test.go +++ b/internal/cli/catalog_test.go @@ -1,3 +1,6 @@ +// SPDX-FileCopyrightText: Copyright 2023 The Minder Authors +// SPDX-License-Identifier: Apache-2.0 + package cli import (