feat(configuration): add git repo as configuration store#4380
Open
CasperGN wants to merge 12 commits into
Open
feat(configuration): add git repo as configuration store#4380CasperGN wants to merge 12 commits into
CasperGN wants to merge 12 commits into
Conversation
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
7 tasks
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new Git-backed configuration store (configuration.git) to components-contrib, plus conformance/certification coverage and CI wiring so the component can be exercised in automated test runs.
Changes:
- Introduces the
configuration/gitimplementation (clone/fetch/poll, mapping modes, auth modes, subscriber fan-out). - Extends conformance + certification test suites to include the new Git configuration store (and a local file:// profile).
- Updates repository Go module versions and dependencies to support the new component and its transitive deps.
Reviewed changes
Copilot reviewed 30 out of 32 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
configuration/git/git.go |
Main Git configuration store implementation (Init/Get/Subscribe/Close, poller orchestration). |
configuration/git/poll.go |
Poll loop, snapshot refresh, diff computation, and subscriber dispatch. |
configuration/git/clone.go |
Clone + fetch/reset helpers using go-git. |
configuration/git/auth.go |
Auth strategies (none, PAT, SSH, GitHub App) for go-git operations. |
configuration/git/subscriber.go |
Subscription state and key filtering helper. |
configuration/git/walk.go |
Worktree file walker with hidden-file rules and file-size cap. |
configuration/git/mapping.go |
Mapping strategies (file, agentYaml, prompty) from files to config items. |
configuration/git/README.md |
Component documentation and behavioral notes/limitations. |
configuration/git/metadata.go |
Metadata parsing/validation (URL, auth, polling, mapping mode, etc.). |
configuration/git/metadata.yaml |
Component metadata schema for docs/tooling. |
configuration/git/auth_test.go |
Unit tests for auth strategy selection and refresh behavior. |
configuration/git/git_test.go |
Unit/integration-style tests for polling, Get/Subscribe, deletion signaling, Close behavior. |
configuration/git/mapping_test.go |
Unit tests for mapping modes and edge cases. |
configuration/git/metadata_test.go |
Unit tests for metadata defaults and validation constraints. |
configuration/git/poll_test.go |
Unit tests for diff behavior and key filtering. |
configuration/git/walk_test.go |
Unit tests for walker hidden/.git exclusion and file-size cap. |
tests/utils/configupdater/git/git.go |
Conformance harness updater that commits/pushes changes to a local upstream repo. |
tests/conformance/configuration/configuration.go |
Conformance harness updated to normalize deletion semantics and “ignore version” behavior for Git. |
tests/conformance/configuration_test.go |
Registers Git store + updater for conformance runs. |
tests/config/configuration/tests.yml |
Adds git.local to the conformance component matrix. |
tests/config/configuration/git/local/configstore.yml |
Conformance component spec for local file:// Git repo. |
tests/certification/configuration/git/git_test.go |
New certification test exercising Get/Subscribe/delete behavior via embedded sidecar. |
tests/certification/configuration/git/config.yaml |
Certification test configuration manifest. |
tests/certification/configuration/git/components/default/configstore.yaml |
Placeholder component spec to satisfy tooling; runtime spec is written by the test. |
.github/scripts/components-scripts/conformance-configuration.git-setup.sh |
CI setup script that creates a local bare repo and exports GIT_UPSTREAM_URL. |
.github/scripts/test-info.mjs |
Adds conformance/certification entries for configuration.git and configuration.git.local. |
.golangci.yml |
Adds prompty to cspell ignore list. |
go.mod |
Bumps Go version and adds go-git + related indirect deps. |
go.sum |
Dependency lockfile updates for the root module. |
tests/certification/go.mod |
Bumps Go version and adds new indirect deps for certification module. |
tests/certification/go.sum |
Dependency lockfile updates for certification module. |
.build-tools/go.mod |
Bumps Go version for build-tools module. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Casper Nielsen <casper@diagrid.io>
…items Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
sicoyle
requested changes
May 11, 2026
Contributor
sicoyle
left a comment
sicoyle
left a comment
There was a problem hiding this comment.
3/31 files reviewed by me so far - thank you!!
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
Signed-off-by: Casper Nielsen <casper@diagrid.io>
JoshVanL
requested changes
May 28, 2026
Contributor
JoshVanL
left a comment
JoshVanL
left a comment
There was a problem hiding this comment.
Please can we evaluate needing to full the entire repo, rather than caching individual files and keeping track of the HEAD sha. Also why we need to be writing to files.
Comment on lines
+257
to
+277
| // parseRSAPrivateKey decodes a PEM-encoded RSA private key. Accepts both | ||
| // PKCS#1 ("RSA PRIVATE KEY") and PKCS#8 ("PRIVATE KEY") encodings — GitHub | ||
| // Apps can be downloaded in either form. | ||
| func parseRSAPrivateKey(pemBytes []byte) (*rsa.PrivateKey, error) { | ||
| block, _ := pem.Decode(pemBytes) | ||
| if block == nil { | ||
| return nil, errors.New("no PEM block found") | ||
| } | ||
| if key, err := x509.ParsePKCS1PrivateKey(block.Bytes); err == nil { | ||
| return key, nil | ||
| } | ||
| parsed, err := x509.ParsePKCS8PrivateKey(block.Bytes) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("parse private key: %w", err) | ||
| } | ||
| rsaKey, ok := parsed.(*rsa.PrivateKey) | ||
| if !ok { | ||
| return nil, fmt.Errorf("expected RSA private key, got %T", parsed) | ||
| } | ||
| return rsaKey, nil | ||
| } |
Comment on lines
+50
to
+74
| // selectAuth resolves the active auth strategy from metadata. log is | ||
| // guaranteed non-nil by callers (it threads through from the Store's | ||
| // constructor, which always supplies a logger). | ||
| func selectAuth(m *metadata, log logger.Logger) (authStrategy, error) { | ||
| switch m.resolveAuthMode() { | ||
| case authModeNone: | ||
| return &noneAuth{}, nil | ||
| case authModePAT: | ||
| return newPATAuth(m), nil | ||
| case authModeSSH: | ||
| return newSSHAuth(m, log) | ||
| case authModeGithubApp: | ||
| return newGitHubAppAuth(m, log, defaultInstallationTokenFetcher) | ||
| } | ||
| return nil, fmt.Errorf("unsupported auth mode %q", m.resolveAuthMode()) | ||
| } | ||
|
|
||
| // --------------- none --------------- | ||
|
|
||
| type noneAuth struct{} | ||
|
|
||
| func (*noneAuth) AuthMethod(context.Context) (transport.AuthMethod, error) { return nil, nil } | ||
| func (*noneAuth) Close() error { return nil } | ||
|
|
||
| // --------------- pat --------------- |
Contributor
There was a problem hiding this comment.
Please break out file into multiple topic separated files in a auth sub package.
Comment on lines
+240
to
+256
| func (a *githubAppAuth) mintJWT() (string, error) { | ||
| now := time.Now() | ||
| tok, err := jwt.NewBuilder(). | ||
| Issuer(strconv.FormatInt(a.appID, 10)). | ||
| IssuedAt(now.Add(-30 * time.Second)). | ||
| Expiration(now.Add(10 * time.Minute)). | ||
| Build() | ||
| if err != nil { | ||
| return "", fmt.Errorf("build jwt: %w", err) | ||
| } | ||
| signed, err := jwt.Sign(tok, jwt.WithKey(jwa.RS256, a.privateKey)) | ||
| if err != nil { | ||
| return "", fmt.Errorf("sign jwt: %w", err) | ||
| } | ||
| return string(signed), nil | ||
| } | ||
|
|
| Auth: method, | ||
| ReferenceName: plumbing.NewBranchReferenceName(branch), | ||
| SingleBranch: true, | ||
| Depth: depth, |
Contributor
There was a problem hiding this comment.
When would this ever be not 1?
| SingleBranch: true, | ||
| Depth: depth, | ||
| } | ||
| repo, err := gogit.PlainCloneContext(ctx, dir, false, opts) |
Contributor
There was a problem hiding this comment.
Why do we want to clone the entire repo, and not just single files?
Comment on lines
+55
to
+70
| func fetchIfChanged(ctx context.Context, repo *gogit.Repository, branch string, auth authStrategy) (plumbing.Hash, bool, error) { | ||
| method, err := auth.AuthMethod(ctx) | ||
| if err != nil { | ||
| return plumbing.ZeroHash, false, fmt.Errorf("auth: %w", err) | ||
| } | ||
| branchRef := plumbing.NewBranchReferenceName(branch) | ||
| remoteRef := plumbing.NewRemoteReferenceName(remoteName, branch) | ||
|
|
||
| fetchSpec := config.RefSpec(fmt.Sprintf("+%s:%s", branchRef, remoteRef)) | ||
| fetchErr := repo.FetchContext(ctx, &gogit.FetchOptions{ | ||
| RemoteName: remoteName, | ||
| Auth: method, | ||
| RefSpecs: []config.RefSpec{fetchSpec}, | ||
| Force: true, | ||
| }) | ||
| upToDate := errors.Is(fetchErr, gogit.NoErrAlreadyUpToDate) |
Contributor
There was a problem hiding this comment.
Is this better than querying the remote and comparing SHAs?
Comment on lines
+110
to
+114
| dir, err := os.MkdirTemp("", "dapr-config-git-") | ||
| if err != nil { | ||
| return fmt.Errorf("create workdir: %w", err) | ||
| } | ||
| s.workdir = dir |
Contributor
There was a problem hiding this comment.
Why to file and not keep in memory?
Comment on lines
+121
to
+125
| _ = os.RemoveAll(dir) | ||
| s.workdir = "" | ||
| if s.auth != nil { | ||
| _ = s.auth.Close() | ||
| } |
Contributor
There was a problem hiding this comment.
Please ensure we are logging errors and not swallowing these in all instances.
Comment on lines
+136
to
+140
| cache, err := lru.New[plumbing.Hash, map[string]*configuration.Item](s.metadata.snapshotCacheSize()) | ||
| if err != nil { | ||
| return fmt.Errorf("init lru: %w", err) | ||
| } | ||
| s.lru = cache |
| pollerCtx, pollerCancel := context.WithCancel(context.Background()) | ||
| s.pollerCancel = pollerCancel | ||
| s.pollerWg.Add(1) | ||
| go s.pollLoop(pollerCtx) |
Contributor
There was a problem hiding this comment.
Wait group go to ensure no go routine leaks.
split auth into sub-package
Comment on lines
+172
to
+182
| // Push both `master` and `main` so we don't fight the locally-default | ||
| // branch. The destination is always `main` on the upstream. | ||
| if err := u.repo.Push(&gogit.PushOptions{ | ||
| RemoteName: "origin", | ||
| RefSpecs: []config.RefSpec{ | ||
| config.RefSpec("refs/heads/master:refs/heads/" + u.branch), | ||
| config.RefSpec("refs/heads/" + u.branch + ":refs/heads/" + u.branch), | ||
| }, | ||
| }); err != nil && !errors.Is(err, gogit.NoErrAlreadyUpToDate) { | ||
| return fmt.Errorf("push: %w", err) | ||
| } |
Comment on lines
+147
to
+150
| require.NoError(u.t, u.repo.Push(&gogit.PushOptions{ | ||
| RemoteName: "origin", | ||
| RefSpecs: []config.RefSpec{config.RefSpec("refs/heads/master:refs/heads/main"), config.RefSpec("refs/heads/main:refs/heads/main")}, | ||
| })) |
Comment on lines
+169
to
+172
| require.NoError(u.t, u.repo.Push(&gogit.PushOptions{ | ||
| RemoteName: "origin", | ||
| RefSpecs: []config.RefSpec{config.RefSpec("refs/heads/master:refs/heads/main"), config.RefSpec("refs/heads/main:refs/heads/main")}, | ||
| })) |
Comment on lines
+157
to
+163
| if when, err := http.ParseTime(v); err == nil { | ||
| if d := time.Until(when); d > 0 { | ||
| return d | ||
| } | ||
| // HTTP-date already in the past — fall through to "no wait". | ||
| _ = now | ||
| } |
Comment on lines
+46
to
+61
| // NOTE: tc.Run(t) was lost in commit 1208b3e3 ("Conformance | ||
| // test: move loader to each component type's folder"), so the | ||
| // configuration conformance suite has been a silent no-op | ||
| // since then — no component has actually exercised these | ||
| // tests in CI. Restoring tc.Run(t) (below) surfaces years of | ||
| // latent failures in redis/postgres/kubernetes that are out | ||
| // of scope for the configuration.git PR. | ||
| // | ||
| // To keep the matrix listing accurate while not blocking on | ||
| // pre-existing breakage, skip non-git components explicitly so | ||
| // CI output reflects what is and isn't being exercised. A | ||
| // dedicated framework-repair pass should remove these skips. | ||
| if comp.Component != "git.local" { | ||
| t.Skipf("configuration conformance for %s is not currently exercised: this suite has been a no-op since commit 1208b3e3; the tc.Run(t) restoration in this PR is scoped to git.local only", comp.Component) | ||
| return | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Enable using git repos as configuration stores. This is especially helpful for
DaprAgentswhere users want to version and treat prompty files or the prompt as a versioned artifact.Issue reference
We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.
Please reference the issue this PR will close: N/A
Checklist
Please make sure you've completed the relevant tasks for this PR, out of the following list:
Note: We expect contributors to open a corresponding documentation PR in the dapr/docs repository. As the implementer, you are the best person to document your work! Implementation PRs will not be merged until the documentation PR is opened and ready for review.