Skip to content

Commit 493ce48

Browse files
reubenoreubeno
authored
feat: add new upstream-commit option for commit-pinning specs (#487)
Allows specifying that an upstream-sourced spec should be fetched from a specific git commit (indicated by hash).
1 parent aecff72 commit 493ce48

File tree

7 files changed

+279
-15
lines changed

7 files changed

+279
-15
lines changed

‎internal/projectconfig/specsource.go‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ type SpecSource struct {
1616

1717
// UpstreamName indicates the name of the component in the upstream distro; only relevant for upstream specs.
1818
UpstreamName string `toml:"upstream-name,omitempty" json:"upstreamName,omitempty" validate:"excluded_unless=SourceType upstream" jsonschema:"title=Upstream component name,description=Name of the component in the upstream distro,example=different-name"`
19+
20+
// UpstreamCommit pins the upstream spec to a specific git commit hash; only relevant for upstream specs.
21+
// When set, this takes priority over the snapshot date-time on the distro reference.
22+
UpstreamCommit string `toml:"upstream-commit,omitempty" json:"upstreamCommit,omitempty" validate:"excluded_unless=SourceType upstream,omitempty,hexadecimal,min=7,max=40" jsonschema:"title=Upstream commit,description=Git commit hash to pin the upstream spec to. Takes priority over snapshot.,minLength=7,maxLength=40,pattern=^[0-9a-fA-F]+$,example=abc1234def5678"`
1923
}
2024

2125
// Implements the [Stringer] interface.
@@ -26,7 +30,12 @@ func (s *SpecSource) String() string {
2630
case SpecSourceTypeLocal:
2731
return s.Path
2832
case SpecSourceTypeUpstream:
29-
return "Upstream: " + s.UpstreamDistro.String()
33+
result := "Upstream: " + s.UpstreamDistro.String()
34+
if s.UpstreamCommit != "" {
35+
result += " @" + s.UpstreamCommit
36+
}
37+
38+
return result
3039
default:
3140
return ""
3241
}

‎internal/projectconfig/specsource_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,3 +49,52 @@ func TestSpecSourceValidation_SourceTargets(t *testing.T) {
4949
SourceType: projectconfig.SpecSourceTypeLocal,
5050
}))
5151
}
52+
53+
func TestSpecSourceValidation_UpstreamCommit(t *testing.T) {
54+
// UpstreamCommit is valid when type is upstream and value is a valid hex hash.
55+
require.NoError(t, validator.New().Struct(&projectconfig.SpecSource{
56+
SourceType: projectconfig.SpecSourceTypeUpstream,
57+
UpstreamCommit: "abc1234",
58+
}))
59+
60+
// Full 40-char SHA is valid.
61+
require.NoError(t, validator.New().Struct(&projectconfig.SpecSource{
62+
SourceType: projectconfig.SpecSourceTypeUpstream,
63+
UpstreamCommit: "abc1234def5678abc1234def5678abc1234def56",
64+
}))
65+
66+
// UpstreamCommit is optional (empty is fine with type=upstream).
67+
require.NoError(t, validator.New().Struct(&projectconfig.SpecSource{
68+
SourceType: projectconfig.SpecSourceTypeUpstream,
69+
}))
70+
71+
// UpstreamCommit is rejected when type is local.
72+
require.Error(t, validator.New().Struct(&projectconfig.SpecSource{
73+
SourceType: projectconfig.SpecSourceTypeLocal,
74+
Path: "/some/path",
75+
UpstreamCommit: "abc1234",
76+
}))
77+
78+
// UpstreamCommit is rejected when type is unspecified.
79+
require.Error(t, validator.New().Struct(&projectconfig.SpecSource{
80+
UpstreamCommit: "abc1234",
81+
}))
82+
83+
// UpstreamCommit is rejected when non-hex.
84+
require.Error(t, validator.New().Struct(&projectconfig.SpecSource{
85+
SourceType: projectconfig.SpecSourceTypeUpstream,
86+
UpstreamCommit: "not-a-hex-value",
87+
}))
88+
89+
// UpstreamCommit is rejected when too short (< 7 chars).
90+
require.Error(t, validator.New().Struct(&projectconfig.SpecSource{
91+
SourceType: projectconfig.SpecSourceTypeUpstream,
92+
UpstreamCommit: "abc12",
93+
}))
94+
95+
// UpstreamCommit is rejected when too long (> 40 chars).
96+
require.Error(t, validator.New().Struct(&projectconfig.SpecSource{
97+
SourceType: projectconfig.SpecSourceTypeUpstream,
98+
UpstreamCommit: "abc1234def5678abc1234def5678abc1234def5678a",
99+
}))
100+
}

‎internal/providers/sourceproviders/fedorasourceprovider.go‎

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,9 @@ func (g *FedoraSourcesProviderImpl) GetComponent(
107107
slog.Info("Getting component from git repo",
108108
"component", componentName,
109109
"upstreamComponent", upstreamNameToUse,
110-
"branch", distroGitBranch)
110+
"branch", distroGitBranch,
111+
"upstreamCommit", component.GetConfig().Spec.UpstreamCommit,
112+
"snapshot", effectiveDistroRef.Snapshot)
111113

112114
// Clone to a temp directory first, then copy files to destination.
113115
tempDir, err := fileutils.MkdirTempInTempDir(g.fs, "azldev-clone-")
@@ -130,19 +132,20 @@ func (g *FedoraSourcesProviderImpl) GetComponent(
130132
}
131133

132134
// Process the cloned repo: checkout target commit, extract sources, copy to destination.
133-
return g.processClonedRepo(ctx, effectiveDistroRef, tempDir, upstreamNameToUse, componentName,
134-
lookasideBaseURI, destDirPath)
135+
return g.processClonedRepo(ctx, effectiveDistroRef, component.GetConfig().Spec.UpstreamCommit,
136+
tempDir, upstreamNameToUse, componentName, lookasideBaseURI, destDirPath)
135137
}
136138

137139
// processClonedRepo handles the post-clone steps: checking out the target commit,
138140
// extracting lookaside sources, renaming spec files, and copying to the destination.
139141
func (g *FedoraSourcesProviderImpl) processClonedRepo(
140142
ctx context.Context,
141143
effectiveDistroRef projectconfig.DistroReference,
144+
upstreamCommit string,
142145
tempDir, upstreamName, componentName, lookasideBaseURI, destDirPath string,
143146
) error {
144147
// Checkout the appropriate commit based on component/distro config
145-
if err := g.checkoutTargetCommit(ctx, effectiveDistroRef, tempDir); err != nil {
148+
if err := g.checkoutTargetCommit(ctx, effectiveDistroRef, upstreamCommit, tempDir); err != nil {
146149
return fmt.Errorf("failed to checkout target commit:\n%w", err)
147150
}
148151

@@ -222,14 +225,28 @@ func (g *FedoraSourcesProviderImpl) resolveDistroConfig(
222225

223226
// checkoutTargetCommit determines the appropriate commit to use and checks it out.
224227
// Priority order:
225-
// 1. Upstream distro snapshot - snapshot time from the effective distro reference
226-
// 2. Default - use current HEAD (no checkout needed)
228+
// 1. Explicit upstream commit hash - specified per-component via upstream-commit
229+
// 2. Upstream distro snapshot - snapshot time from the effective distro reference
230+
// 3. Default - use current HEAD (no checkout needed)
227231
func (g *FedoraSourcesProviderImpl) checkoutTargetCommit(
228232
ctx context.Context,
229233
effectiveDistroRef projectconfig.DistroReference,
234+
upstreamCommit string,
230235
repoDir string,
231236
) error {
232-
// Case 1: Effective distro reference has snapshot time configured
237+
// Case 1: Explicit upstream commit hash specified per-component
238+
if upstreamCommit != "" {
239+
slog.Info("Using explicit upstream commit hash",
240+
"commitHash", upstreamCommit)
241+
242+
if err := g.gitProvider.Checkout(ctx, repoDir, upstreamCommit); err != nil {
243+
return fmt.Errorf("failed to checkout upstream commit %#q:\n%w", upstreamCommit, err)
244+
}
245+
246+
return nil
247+
}
248+
249+
// Case 2: Effective distro reference has snapshot time configured
233250
if snapshotStr := effectiveDistroRef.Snapshot; snapshotStr != "" {
234251
snapshotDateTime, err := time.Parse(time.RFC3339, snapshotStr)
235252
if err != nil {
@@ -242,7 +259,7 @@ func (g *FedoraSourcesProviderImpl) checkoutTargetCommit(
242259
snapshotDateTime.Format(time.RFC3339), err)
243260
}
244261

245-
slog.Debug("Using upstream distro snapshot time",
262+
slog.Info("Using upstream distro snapshot time",
246263
"snapshotDateTime", snapshotDateTime.Format(time.RFC3339),
247264
"commitHash", commitHash)
248265

@@ -253,8 +270,8 @@ func (g *FedoraSourcesProviderImpl) checkoutTargetCommit(
253270
return nil
254271
}
255272

256-
// Case 2: Default - use current HEAD (already checked out by clone)
257-
slog.Debug("Using current HEAD (no snapshot time configured)")
273+
// Case 3: Default - use current HEAD (already checked out by clone)
274+
slog.Info("Using current HEAD (no snapshot time configured)")
258275

259276
return nil
260277
}

‎internal/providers/sourceproviders/fedorasourceprovider_test.go‎

Lines changed: 160 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,11 @@ import (
2727
)
2828

2929
const (
30-
destDir = "/output"
31-
repoURL = "https://example.com/" + testPackageName + ".git"
32-
branch = "main"
33-
distGitBaseURI = "https://example.com/$pkg.git"
30+
destDir = "/output"
31+
repoURL = "https://example.com/" + testPackageName + ".git"
32+
branch = "main"
33+
distGitBaseURI = "https://example.com/$pkg.git"
34+
testUpstreamCommit = "abc1234def5678"
3435
)
3536

3637
// mockDistroResolver returns a mock distro resolver that returns the default test distro config.
@@ -732,3 +733,158 @@ func TestCheckoutTargetCommit(t *testing.T) {
732733
assert.Contains(t, err.Error(), "invalid snapshot time")
733734
})
734735
}
736+
737+
func TestCheckoutTargetCommit_UpstreamCommit(t *testing.T) {
738+
env := testutils.NewTestEnv(t)
739+
740+
t.Run("uses upstream commit when configured", func(t *testing.T) {
741+
ctrl := gomock.NewController(t)
742+
mockGitProvider := git_test.NewMockGitProvider(ctrl)
743+
mockExtractor := fedorasource_test.NewMockFedoraSourceDownloader(ctrl)
744+
745+
upstreamCommitHash := testUpstreamCommit
746+
747+
provider, err := sourceproviders.NewFedoraSourcesProviderImpl(
748+
env.FS(),
749+
env.DryRunnable,
750+
mockGitProvider,
751+
mockExtractor,
752+
mockDistroResolver(),
753+
retry.Disabled(),
754+
)
755+
require.NoError(t, err)
756+
757+
mockComponent := components_testutils.NewMockComponent(ctrl)
758+
mockComponent.EXPECT().GetName().AnyTimes().Return(testPackageName)
759+
mockComponent.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{
760+
Name: testPackageName,
761+
Spec: projectconfig.SpecSource{
762+
UpstreamCommit: upstreamCommitHash,
763+
},
764+
})
765+
766+
// Clone succeeds
767+
mockGitProvider.EXPECT().
768+
Clone(gomock.Any(), repoURL, gomock.Any(), gomock.Any()).
769+
DoAndReturn(func(ctx context.Context, repoURL, cloneDir string, opts ...git.GitOptions) error {
770+
specPath := cloneDir + "/" + testPackageName + ".spec"
771+
772+
return fileutils.WriteFile(env.FS(), specPath, []byte("Name: "+testPackageName), fileperms.PublicFile)
773+
})
774+
775+
// Should checkout the explicit commit hash
776+
mockGitProvider.EXPECT().
777+
Checkout(gomock.Any(), gomock.Any(), upstreamCommitHash).
778+
Return(nil)
779+
780+
// Should NOT call GetCommitHashBeforeDate
781+
// (gomock will fail if it's called since we didn't set an expectation)
782+
783+
// Extractor succeeds
784+
mockExtractor.EXPECT().
785+
ExtractSourcesFromRepo(gomock.Any(), gomock.Any(), testPackageName, gomock.Any()).
786+
Return(nil)
787+
788+
err = provider.GetComponent(context.Background(), mockComponent, destDir)
789+
require.NoError(t, err)
790+
})
791+
792+
t.Run("upstream commit takes priority over snapshot", func(t *testing.T) {
793+
ctrl := gomock.NewController(t)
794+
mockGitProvider := git_test.NewMockGitProvider(ctrl)
795+
mockExtractor := fedorasource_test.NewMockFedoraSourceDownloader(ctrl)
796+
797+
upstreamCommitHash := testUpstreamCommit
798+
snapshotTimeStr := "2026-01-10T11:11:30-08:00"
799+
800+
provider, err := sourceproviders.NewFedoraSourcesProviderImpl(
801+
env.FS(),
802+
env.DryRunnable,
803+
mockGitProvider,
804+
mockExtractor,
805+
mockDistroResolver(),
806+
retry.Disabled(),
807+
)
808+
require.NoError(t, err)
809+
810+
mockComponent := components_testutils.NewMockComponent(ctrl)
811+
mockComponent.EXPECT().GetName().AnyTimes().Return(testPackageName)
812+
mockComponent.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{
813+
Name: testPackageName,
814+
Spec: projectconfig.SpecSource{
815+
UpstreamCommit: upstreamCommitHash,
816+
UpstreamDistro: projectconfig.DistroReference{
817+
Name: "fedora",
818+
Snapshot: snapshotTimeStr,
819+
},
820+
},
821+
})
822+
823+
// Clone succeeds
824+
mockGitProvider.EXPECT().
825+
Clone(gomock.Any(), repoURL, gomock.Any(), gomock.Any()).
826+
DoAndReturn(func(ctx context.Context, repoURL, cloneDir string, opts ...git.GitOptions) error {
827+
specPath := cloneDir + "/" + testPackageName + ".spec"
828+
829+
return fileutils.WriteFile(env.FS(), specPath, []byte("Name: "+testPackageName), fileperms.PublicFile)
830+
})
831+
832+
// Should use the explicit commit, NOT the snapshot
833+
mockGitProvider.EXPECT().
834+
Checkout(gomock.Any(), gomock.Any(), upstreamCommitHash).
835+
Return(nil)
836+
837+
// Should NOT call GetCommitHashBeforeDate (snapshot is ignored)
838+
839+
// Extractor succeeds
840+
mockExtractor.EXPECT().
841+
ExtractSourcesFromRepo(gomock.Any(), gomock.Any(), testPackageName, gomock.Any()).
842+
Return(nil)
843+
844+
err = provider.GetComponent(context.Background(), mockComponent, destDir)
845+
require.NoError(t, err)
846+
})
847+
848+
t.Run("upstream commit checkout failure propagates", func(t *testing.T) {
849+
ctrl := gomock.NewController(t)
850+
mockGitProvider := git_test.NewMockGitProvider(ctrl)
851+
mockExtractor := fedorasource_test.NewMockFedoraSourceDownloader(ctrl)
852+
853+
upstreamCommitHash := testUpstreamCommit
854+
855+
provider, err := sourceproviders.NewFedoraSourcesProviderImpl(
856+
env.FS(),
857+
env.DryRunnable,
858+
mockGitProvider,
859+
mockExtractor,
860+
mockDistroResolver(),
861+
retry.Disabled(),
862+
)
863+
require.NoError(t, err)
864+
865+
mockComponent := components_testutils.NewMockComponent(ctrl)
866+
mockComponent.EXPECT().GetName().AnyTimes().Return(testPackageName)
867+
mockComponent.EXPECT().GetConfig().AnyTimes().Return(&projectconfig.ComponentConfig{
868+
Name: testPackageName,
869+
Spec: projectconfig.SpecSource{
870+
UpstreamCommit: upstreamCommitHash,
871+
},
872+
})
873+
874+
// Clone succeeds
875+
mockGitProvider.EXPECT().
876+
Clone(gomock.Any(), repoURL, gomock.Any(), gomock.Any()).
877+
Return(nil)
878+
879+
// Checkout fails
880+
checkoutError := errors.New("commit not found")
881+
mockGitProvider.EXPECT().
882+
Checkout(gomock.Any(), gomock.Any(), upstreamCommitHash).
883+
Return(checkoutError)
884+
885+
err = provider.GetComponent(context.Background(), mockComponent, destDir)
886+
require.Error(t, err)
887+
assert.Contains(t, err.Error(), "failed to checkout upstream commit")
888+
assert.ErrorIs(t, err, checkoutError)
889+
})
890+
}

‎scenario/__snapshots__/TestSnapshotsContainer_config_generate-schema_stdout_1.snap‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,17 @@
586586
"examples": [
587587
"different-name"
588588
]
589+
},
590+
"upstream-commit": {
591+
"type": "string",
592+
"maxLength": 40,
593+
"minLength": 7,
594+
"pattern": "^[0-9a-fA-F]+$",
595+
"title": "Upstream commit",
596+
"description": "Git commit hash to pin the upstream spec to. Takes priority over snapshot.",
597+
"examples": [
598+
"abc1234def5678"
599+
]
589600
}
590601
},
591602
"additionalProperties": false,

‎scenario/__snapshots__/TestSnapshots_config_generate-schema_stdout_1.snap‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,17 @@
586586
"examples": [
587587
"different-name"
588588
]
589+
},
590+
"upstream-commit": {
591+
"type": "string",
592+
"maxLength": 40,
593+
"minLength": 7,
594+
"pattern": "^[0-9a-fA-F]+$",
595+
"title": "Upstream commit",
596+
"description": "Git commit hash to pin the upstream spec to. Takes priority over snapshot.",
597+
"examples": [
598+
"abc1234def5678"
599+
]
589600
}
590601
},
591602
"additionalProperties": false,

‎schemas/azldev.schema.json‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,17 @@
586586
"examples": [
587587
"different-name"
588588
]
589+
},
590+
"upstream-commit": {
591+
"type": "string",
592+
"maxLength": 40,
593+
"minLength": 7,
594+
"pattern": "^[0-9a-fA-F]+$",
595+
"title": "Upstream commit",
596+
"description": "Git commit hash to pin the upstream spec to. Takes priority over snapshot.",
597+
"examples": [
598+
"abc1234def5678"
599+
]
589600
}
590601
},
591602
"additionalProperties": false,

0 commit comments

Comments
 (0)