feat(cli): make quickstart catalog repository configurable via --catalog-repo flag by sachin9058 · Pull Request #6364 · mindersec/minder

sachin9058 · 2026-04-14T12:55:50Z

Summary

Add a --catalog-repo flag to the quickstart command to allow users to specify a custom repository for loading rule and profile catalogs.

Previously, the catalog repository URL was hardcoded, limiting flexibility to the default Minder catalog. This change enables users to provide their own repository while preserving the existing default behavior.

If the flag is not provided, the quickstart flow continues to use the default Minder catalog repository.

Additionally, the help text has been improved to better describe the purpose and usage of the flag.

Testing

Ran:
```
make lint-fix
go test ./...
```
Verified CLI flag parsing by running:
go run ./cmd/cli quickstart --catalog-repo
Confirmed default behavior remains unchanged when no flag is provided
Confirmed custom repository value is correctly read and passed into the catalog loading flow
Verified fallback logic remains intact when repository loading fails

…log-repo flag

coveralls · 2026-04-14T14:53:53Z

Coverage is 60.268% — sachin9058:feat/quickstart-configurable-repo into mindersec:main. No base build found for mindersec:main.

evankanderson · 2026-04-14T15:05:34Z

@evankanderson This PR depends on the in-memory repository clone utility introduced in #6357.

The quickstart changes use the internal/repo package from that PR. Once it is merged, CI/build issues related to missing imports will be resolved.

The way to do this is to git rebase this branch onto that one.

Thinking about the amount of code and a desire to test it, we should probably put the clone and find directories code in internal/cli.

sachin9058 · 2026-04-14T15:12:13Z

@evankanderson

@evankanderson This PR depends on the in-memory repository clone utility introduced in #6357.
The quickstart changes use the internal/repo package from that PR. Once it is merged, CI/build issues related to missing imports will be resolved.

The way to do this is to git rebase this branch onto that one.

Thinking about the amount of code and a desire to test it, we should probably put the clone and find directories code in internal/cli.

Got it, that makes sense thanks!

I’ve moved away from depending on #6357 and implemented the clone logic inline for now so the PR is self-contained.

That said, I agree it would be better to structure this under internal/cli for clarity and testability. I’ll refactor the current implementation accordingly.

Let me update the PR with that approach 👍

…quickstart from clone helper

evankanderson

Rather than just converting the existing rule loading, we should probably do something more extensive where we load a profile, ruletypes, and supporting datasources from one of the README enabled directories in minder-rules-and-profiles?

That way, we can include the description of what the profile will do in the guided setup.

sachin9058 · 2026-04-15T04:16:15Z

@evankanderson That makes sense, I like that direction 👍

Right now I focused on keeping the existing quickstart behavior but sourcing the rule type and profile from the repo.

Extending this to support README-based directories with profiles, rule types, and datasources would definitely make the flow more flexible and user-friendly.

I can open a follow-up PR to explore that approach once this one is in, if that sounds good.

evankanderson

I think there's a bit more refactoring we could do to lean towards loading files from a filesystem (if we package it right, we could even use https://pkg.go.dev/embed#hdr-File_Systems to supply a backup set of rules that don't need to clone).

… for quickstart Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

sachin9058 · 2026-04-22T22:09:04Z

@evankanderson Thanks for the detailed feedback — I’ve reworked the implementation accordingly.

Switched to filesystem-driven loading for rule types and profiles from the catalog repository
Added validation to ensure profiles only reference existing rule types
Removed the previous hardcoded/single-file approach
Structured the flow as load → validate → prompt → apply
Made the apply step transactional with rollback on failure

I also resolved the merge conflicts and cleaned up the commit history to keep this PR focused on the quickstart changes.

Please let me know if you’d like me to refine anything further or adjust the approach.

evankanderson

I'm not sure exactly what behavior we're expecting here -- it seems like the current code will try to load all the generic ruletypes and profiles from minder-rules-and-profiles into one project, which seems like it might be overload for a new user.

Additionally, if this work is laying the foundation for future refinement, I'd expect it to be adjusting / improving existing code (e.g. leveraging the same abstractions as the apply and create CLI commands), rather than implementing a lot of heavy new logic in cmd/. (Quickstart already has a bunch of heavy prompting logic, but it would be great to reduce the amount of logic in cmd/ (which doesn't contribute to coverage numbers), and increase the logic in internal/util/cli and pkg/.

sachin9058 · 2026-04-23T08:21:53Z

@evankanderson Thanks for the detailed feedback — I’ve made a pass to align the implementation accordingly.

Moved catalog loading and validation into internal/cli, keeping cmd/quickstart focused on orchestration
Reused existing utilities (profiles.ReadProfileFromPath, fileconvert, billyutil.Walk) instead of custom logic
Simplified data structures and loops, and cleaned up error handling
Updated behavior to skip invalid resources with warnings rather than failing the entire load
Removed unnecessary checks and made resource handling consistent

I’ve kept the current loading behavior as-is for now and can iterate on making it more selective in a follow-up PR if that direction makes sense.

Let me know if there’s anything else you’d like refined.

evankanderson

This is looking better, but I think with a few updates to fileconvert, this could be even simpler.

…oading

sachin9058 · 2026-04-23T17:19:00Z

@evankanderson Thanks I’ve updated the implementation to better align with the existing abstractions.

Replaced custom file traversal and parsing with a reusable ResourcesFromFilesystem helper in fileconvert
Reduced duplication in catalog loading by reusing that helper for both rule types and profiles
Moved validation logic into a method on Catalog
Kept CLI logic focused on orchestration

Let me know if you’d prefer further alignment with existing helpers like ResourcesFromPaths, or if this direction looks good.

evankanderson

Some additional suggested changes -- the overall goal is to reduce the amount of code that needs to be maintained / fixed in the future while increasing its capabilities -- right now this is about +250 lines of non-test library code, but I think we can get it closer to +100 while supporting adding repository load for additional sub-commands in a subsequent PR.

…oder Restore absolute path normalization with osfs.New("/") after testing showed regression with osfs.New("."). This ensures existing fileconvert tests and relative path handling continue to work as expected. Also includes prior refactors to unify filesystem expansion and remove duplication.

sachin9058 · 2026-04-23T22:45:15Z

@evankandersonThanks again for the detailed feedback this was very helpful in shaping the final direction.

I’ve made the following updates:

Moved filesystem expansion fully into util.ExpandFileArgs and removed duplicate traversal logic
Simplified collect.go to rely on the unified expansion flow
Removed the unused ReadProfileFromPath helper and related import
Fixed decoder path handling to preserve previous behavior for OS-backed paths
Verified all affected packages with tests and lint (go test and make lint-fix both pass)

Let me know if this looks aligned now or if you’d like any further adjustments.

…ble-repo

…e into main PR - Converted tests to table-driven format - Added behavior-based assertions for profile filtering - Covered empty, valid, and mixed catalog scenarios - Aligned with project testing patterns

sachin9058 · 2026-04-23T22:57:36Z

@evankanderson

I’ve folded the test coverage into this PR so implementation and tests can be reviewed together. I also refactored the tests into a table-driven format to align with existing patterns and added behavior-focused assertions for profile validation.

This should also make it easier to evaluate coverage impact as part of this PR.

Let me know if this looks good or if you’d like any further refinements.

sachin9058 · 2026-04-25T05:55:37Z

Hi @evankanderson can u take a look on this again i have made the changes that aligns with your feedback tell me if it looks good or need more refinements or improvements ??

github-actions · 2026-05-26T05:31:32Z

This PR needs additional information before we can continue. It is now marked as stale because it has been open for 30 days with no activity. Please provide the necessary details to continue or it will be closed in 30 days.

evankanderson · 2026-05-26T18:32:23Z

Sorry -- I somehow missed that this had more updates. I'm taking a look now.

evankanderson

Sorry for the delay getting back to you -- it seems like there were a number of comments which got closed as "resolved" without either action or a counter-argument ("this seems clearer" or "I don't understand the suggestion" would be okay if that's the case).

evankanderson · 2026-05-26T18:54:06Z

 		f = filepath.Clean(f)
-		fi, err := os.Stat(f)
+
+		if vfs == nil {


Rather than creating two implementations within this function, try one of the following:

Use osfs.New() to create a billy filesystem rooted in the current directory:

if vfs == nil { vfs = osfs.New(".", osfs.WithBoundOS()) }

You can convert both types to a base library io/fs FS interface using billy's iofs.New() or os's DirFS() and then calling io/fs's WalkDir()

var fs fs.FS if vfs != nil { fs = iofs.New(vfs) } else { fs = os.DirFS(".") }

This implementation will allow reading absolute symlinks outside the current directory, while the billy implementation will perform a "soft" chroot behavior where it will block reads from outside the directory. I don't have a strong feeling between the two approaches.

In general, when you find that you need two parallel implementations that are more than about 5 lines long, it's generally worth searching for a Go interface you can use to unify the two code paths.

evankanderson · 2026-05-26T18:57:57Z

-	"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"


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.

evankanderson · 2026-05-26T18:59:00Z

-//go:embed embed*
-var content embed.FS
-


Can we remove the embed directory as well if we don't need this //go:embed variable?

evankanderson · 2026-05-26T19:00:33Z

@@ -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


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?

evankanderson · 2026-05-26T19:01:00Z

+const (
+	defaultQuickstartCatalogRepoURL = "https://github.com/mindersec/minder-rules-and-profiles"
+)


Slight preference to have constants at the top of the file, or at least before use.

evankanderson · 2026-05-26T20:05:12Z

+	}
+}
+
+func decoderForPath(fs billy.Filesystem, path string) (Decoder, io.Closer) {


Given that this is a private function, it seems like we can assume that fs has been created correctly previously, no?

evankanderson · 2026-05-26T20:06:38Z

-	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.


The previous code was structured to only attempt to open the file if it was of the appropriate type. This code tries to open the file first, which generates more work and cleanup than the reverse approach.

We've also added an "error" to the return type from decoderForReader, which means we need to check and handle some more cases.

evankanderson · 2026-05-26T20:09:07Z

+func DecoderForFile(path string) (Decoder, io.Closer) {
+	return decoderForPath(nil, path)
+}


Can we simply update existing callers to the new method (and enforce they provide a vfs)?

evankanderson · 2026-05-26T20:10:35Z

+// ReadResourceFromFile reads a single resource from the specified filesystem path.
+func ReadResourceFromFile[T proto.Message](fs billy.Filesystem, path string) (resource T, err error) {


Is this method used anywhere?

evankanderson · 2026-05-26T20:11:30Z

 	defer file.Close()

 	// parse profile from YAML
-	profile, err := profiles.ParseYAML(file)


Did we remove all usage of profiles.ParseYAML in favor of ReadResourceTyped? If so, can we remove the method?

feat(cli): make quickstart catalog repository configurable via --cata…

ddf58be

…log-repo flag

sachin9058 requested a review from a team as a code owner April 14, 2026 12:55

lint-fixed

9971fda

refactor(cli): move catalog clone logic to internal/cli and decouple …

726c1ef

…quickstart from clone helper

evankanderson reviewed Apr 15, 2026

View reviewed changes

evankanderson mentioned this pull request Apr 21, 2026

feat(cli): load quickstart catalogs using in-memory repository clone #6365

Open

evankanderson reviewed Apr 22, 2026

View reviewed changes

feat(cli): implement filesystem-driven catalog loading and validation…

b1766e1

… for quickstart Signed-off-by: Sachin Kumar <sachinkumar905846@gmail.com>

sachin9058 force-pushed the feat/quickstart-configurable-repo branch 2 times, most recently from a25afdc to b1766e1 Compare April 22, 2026 21:50

Merge upstream/main into feat/quickstart-configurable-repo

05441ff

evankanderson reviewed Apr 23, 2026

View reviewed changes

refactor(cli): align quickstart catalog loading with review feedback

e9718ac

evankanderson reviewed Apr 23, 2026

View reviewed changes

Comment thread pkg/profiles/util.go Outdated

Comment thread internal/cli/catalog.go Outdated

Comment thread internal/cli/catalog.go Outdated

Comment thread internal/cli/catalog.go Outdated

Comment thread pkg/profiles/util.go Outdated

sachin9058 and others added 3 commits April 23, 2026 22:45

refactor(cli): reuse fileconvert resources loader and align catalog l…

0891c99

…oading

chore(cli): fix lint issues and polish catalog loading implementation

ece5e51

Merge branch 'main' into feat/quickstart-configurable-repo

55781c9

sachin9058 requested a review from evankanderson April 23, 2026 17:32

test(cli): add coverage for catalog validation edge cases

a061c15

sachin9058 mentioned this pull request Apr 23, 2026

test(cli): add coverage for catalog validation edge cases #6413

Closed

Add SPDX header to catalog tests

e726e90

evankanderson reviewed Apr 23, 2026

View reviewed changes

sachin9058 added 2 commits April 24, 2026 04:20

Merge branch 'test/catalog-loading-v2' into feat/quickstart-configura…

30279ec

…ble-repo

test(cli): add table-driven tests for catalog validation and integrat…

2c0dc21

…e into main PR - Converted tests to table-driven format - Added behavior-based assertions for profile filtering - Covered empty, valid, and mixed catalog scenarios - Aligned with project testing patterns

Merge branch 'main' into feat/quickstart-configurable-repo

3e604df

github-actions Bot added the Stale label May 26, 2026

evankanderson reviewed May 26, 2026

View reviewed changes

github-actions Bot removed the Stale label May 27, 2026

		// ReadResourceFromFile reads a single resource from the specified filesystem path.
		func ReadResourceFromFile[T proto.Message](fs billy.Filesystem, path string) (resource T, err error) {

Conversation

sachin9058 commented Apr 14, 2026

Summary

Testing

Uh oh!

coveralls commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evankanderson commented Apr 14, 2026

Uh oh!

sachin9058 commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

sachin9058 commented Apr 15, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 22, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 23, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 23, 2026

Uh oh!

evankanderson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sachin9058 commented Apr 23, 2026

Uh oh!

sachin9058 commented Apr 23, 2026

Uh oh!

sachin9058 commented Apr 25, 2026

Uh oh!

github-actions Bot commented May 26, 2026

Uh oh!

evankanderson commented May 26, 2026

coveralls commented Apr 14, 2026 •

edited

Loading

sachin9058 commented Apr 14, 2026 •

edited

Loading