feat: introduce Source-based config architecture (new_config, new_bo, new_authmgr, new_db, new_profile) by apuchmarcos · Pull Request #31 · AmadeusITGroup/CDS

apuchmarcos · 2026-02-23T13:50:33Z

Summary

Introduces the foundational new_* packages for the Source-based config architecture refactor. These packages are created alongside the existing ones — no consumers are migrated yet, no existing code is modified.

CDS currently hardcodes local filesystem access (~/.xcds/) across every package. This refactor introduces a Source interface as a transport abstraction so config can eventually be backed by localFS, git, S3, CyberArk, etc.

What's included

new_config — Source interface & root config

Source interface (Read, Write, Exists, Delete) with LocalFSSource implementation
SourceRef struct (type + path) for inline JSON references
CdsConfig root config pointing to 4 sources: inventory, auth, profile, recipes
LoadRootConfig(), SaveRootConfig(), DefaultConfig(), LoadOrCreateRootConfig()

new_bo — Business objects

Host → Agent → Project hierarchy (replaces flat db.json model)
Credential with typed variants: SSH, Password, Token, TLS
Container, Inventory, AuthStore value types
Project ID field (auto-generated as name-<hex>) with globally unique names

new_authmgr — Credential store & resolver

Store: Source-backed credential CRUD with auto-save on mutations
Resolver: typed lookups (ResolveSSH, ResolvePassword, ResolveToken, ResolveTLS)
PromptPassword / PromptAndSavePassword for interactive input

new_db — Inventory manager

InventoryManager: Source-backed Host/Project/Container CRUD
Project names globally unique across all hosts — only AddProject requires hostName, all other ops use just projectName
Host default management, orchestration/registry setters, container operations

new_profile — Profile stub

ProfileManager with Source-backed Load/Save
Profile struct with placeholder Holder field (spec TBD)

Key design decisions

All JSON — no YAML, no Viper
No backwards compatibility — no existing users, no file migration
Credential refs by string key — inventory hosts reference auth entries by key
afero-based testing — all tests use cos.SetMockedFileSystem() for in-memory FS

Related issues

Relates-to feat: Add CLI .config file #20

Type of change

How to test

go build ./...
go test ./internal/new_config/... -v -count=1
go test ./internal/new_authmgr/... -v -count=1
go test ./internal/new_db/... -v -count=1
go test ./internal/new_profile/... -v -count=1

Package	Tests
`new_config`	17
`new_authmgr`	18
`new_db`	49
`new_profile`	4
Total	88

Checklist

I ran make test (or equivalent) and it passed.
I ran make lint (if applicable) and it passed.
I updated docs (README/CONTRIBUTING) if needed.
I added or updated tests where appropriate.
I linked relevant issues and provided context.

Notes for reviewers

All new_* packages are additive only — zero changes to existing code, zero risk to current functionality.
The new_ prefix is temporary. Phase 7 will delete old packages and rename these to their final names.
new_profile is intentionally a stub (Profile struct with a single Holder field). The spec will be defined separately.
Next steps: wire consumers to use new packages and delete old packages, rename new_*.

…db and profile packages

1atsavignac · 2026-02-23T14:04:05Z

internal/new_authmgr/interactive.go

+
+	"github.com/amadeusitgroup/cds/internal/cerr"
+	cg "github.com/amadeusitgroup/cds/internal/global"
+	nc "github.com/amadeusitgroup/cds/internal/new_bo"


What does nc stands for ?
Aliases should be used as little as possible

1atsavignac · 2026-02-23T14:15:40Z

internal/new_authmgr/store.go

+	source nfg.Source
+	ref    nfg.SourceRef


Why do you need both ?
I would think that we have a generic handler that uses the ref type and gets the appropriate CRUD(er ?) through a factory

1atsavignac · 2026-02-23T14:24:18Z

internal/new_authmgr/store.go

+	exists, err := s.source.Exists(s.ref.Path)
+	if err != nil {
+		return fmt.Errorf("failed to check auth file existence: %w", err)
+	}
+	if !exists {
+		s.data = nc.NewAuthStore()
+		return nil
+	}
+
+	raw, err := s.source.Read(s.ref.Path)
+	if err != nil {
+		return fmt.Errorf("failed to read auth file at %s: %w", s.ref.Path, err)
+	}
+
+	var store nc.AuthStore
+	if err := json.Unmarshal(raw, &store); err != nil {
+		return fmt.Errorf("failed to parse auth file at %s: %w", s.ref.Path, err)
+	}


It's even weird to use.
The biggest red flag for me is that the interface Source is from nfg package where in general a package should use interface to "mold" their input effectively constraining the methods instead of the type/fields

source := xxx.getSourceFromRef(s.ref) if source.Exists () {} // or source.exists not sure what's best I'd guess the method store := loadStore(source) // essentially json.Unmarshal(source.Read(), &store)

the source could be even stored at New

func NewStore(ref nfg.SourceRef) *Store { return &Store{ source: xxx.getSourceFromRef(ref), ref: ref, data: nc.NewAuthStore(), } }

You even created it with ResolveSource after

1atsavignac · 2026-02-23T14:33:03Z

internal/new_bo/agent.go

+type Agent struct {
+	Address       string    `json:"address"`
+	CredentialRef string    `json:"credentialRef,omitempty"`
+	Projects      []Project `json:"projects,omitempty"`


Hmm not convinced about this, the Agent is only characterized by how the client speaks to it, the project it host should either come from the agent itself reporting through a gRPC call. You may think about caching/storing the association but elsewhere like a project file like we have now saying which agent it used referenced by address I suppose.

1atsavignac · 2026-02-23T14:34:59Z

internal/new_config/config_test.go

+		Profile:   SourceRef{Type: SourceTypeLocalFS, Path: "/tmp/testcds/.xcds/profile.json"},
+		Recipes:   SourceRef{Type: SourceTypeLocalFS, Path: "/tmp/testcds/.xcds/recipes/"},


having the same type for local file and dir is a dangerous game !

1atsavignac · 2026-02-23T14:45:09Z

internal/new_db/inventory.go

+// Returns pointers to the owning host and the project, or nil if not found.
+// Caller must hold m.mu.
+func (m *InventoryManager) findProjectGlobal(projectName string) (*nb.Host, *nb.Project) {
+	for i := range m.data.Hosts {


Suggested change

for i := range m.data.Hosts {

for i := range (m.data.Hosts) {

1atsavignac · 2026-02-23T14:45:21Z

internal/new_db/inventory.go

+		if h.Agent == nil {
+			continue
+		}
+		for j := range h.Agent.Projects {


Suggested change

for j := range h.Agent.Projects {

for j := range (h.Agent.Projects) {

1atsavignac · 2026-02-23T14:47:26Z

internal/new_db/project.go

+
+	// Auto-generate ID if empty
+	if project.ID == "" {
+		project.ID = generateProjectID(project.Name)


You also need to check that projectID doesn't already exists

1atsavignac · 2026-02-23T14:50:10Z

internal/new_db/project.go

+	for i := range m.data.Hosts {
+		h := &m.data.Hosts[i]
+		if h.Agent == nil {
+			continue
+		}
+		for j := range h.Agent.Projects {
+			if h.Agent.Projects[j].Name == projectName {
+				h.Agent.Projects = append(h.Agent.Projects[:j], h.Agent.Projects[j+1:]...)
+				return nil
+			}
+		}
+	}


I just read what this is doing and get the projects from a call would remove this part altogether and moves it on agent side.

Suggested change

for i := range m.data.Hosts {

h := &m.data.Hosts[i]

if h.Agent == nil {

continue

}

for j := range h.Agent.Projects {

if h.Agent.Projects[j].Name == projectName {

h.Agent.Projects = append(h.Agent.Projects[:j], h.Agent.Projects[j+1:]...)

return nil

}

}

}

for i := range len(m.data.Hosts) {

h := &m.data.Hosts[i]

if h.Agent == nil {

continue

}

for j := range len(h.Agent.Projects) {

if h.Agent.Projects[j].Name == projectName {

h.Agent.Projects = append(h.Agent.Projects[:j], h.Agent.Projects[j+1:]...)

return nil

}

}

}

for h := range m.data.Hosts { slices.DeleteFunc(h.Agent.Projects, func(p project) { return p.name == projectName}) }

if agent ==nil I sure hope len(projects) == 0 😆

1atsavignac · 2026-02-23T15:02:20Z

internal/new_db/project.go

This file is to be reviewed the functions are really funky and I don't think storing everything by host is the best Idea because effectively host and agent are /should be extremely close to each other.
The difference is just that Host is the target to spawn the agent, anything beyond needs to be equivalent

apuchmarcos · 2026-02-25T09:33:50Z

Closed PR to properly split in smaller PRs so its easier to review and follow

Feat: Refactor entire CDS config. Introduce new authmgr, bo, config, …

2d965b6

…db and profile packages

apuchmarcos requested a review from a team as a code owner February 23, 2026 13:50

1atsavignac requested changes Feb 23, 2026

View reviewed changes

apuchmarcos closed this Feb 25, 2026

		Profile: SourceRef{Type: SourceTypeLocalFS, Path: "/tmp/testcds/.xcds/profile.json"},
		Recipes: SourceRef{Type: SourceTypeLocalFS, Path: "/tmp/testcds/.xcds/recipes/"},

	for i := range m.data.Hosts {
	for i := range (m.data.Hosts) {

	for j := range h.Agent.Projects {
	for j := range (h.Agent.Projects) {

Conversation

apuchmarcos commented Feb 23, 2026

Summary

What's included

Key design decisions

Related issues

Type of change

How to test

Checklist

Notes for reviewers

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apuchmarcos commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants