Skip to content

Commit 3e3c797

Browse files
Fix CloudProfile API rejection by skipping invalid SemVer legacy tags (#34)
* fix * upd * fix checks * fix typos check * remove .claude * remove unused license * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix copilot's mistake * fix typo --------- Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent e8ce62d commit 3e3c797

8 files changed

Lines changed: 177 additions & 152 deletions

File tree

.claude/settings.local.json

Lines changed: 0 additions & 15 deletions
This file was deleted.

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,3 +2,5 @@
22
build/
33

44
.idea
5+
6+
.claude

.golangci.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ linters:
136136
# for github.com/sapcc/vpa_butler
137137
- k8s.io/client-go
138138
toolchain-forbidden: true
139-
go-version-pattern: 1\.\d+(\.0)?$
139+
go-version-pattern: 1\.\d+(\.\d+)?$ # manually edited, as default rule does not allow go version with patch, but some deps require e.g. go 1.26.2
140140
gosec:
141141
excludes:
142142
# gosec wants us to set a short ReadHeaderTimeout to avoid Slowloris attacks, but doing so would expose us to Keep-Alive race conditions (see https://iximiuz.com/en/posts/reverse-proxy-http-keep-alive-and-502s/

.typos.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44

55
[default.extend-words]
66

7+
[default]
8+
extend-ignore-identifiers-re = ["ANDed"]
9+
710
[files]
811
extend-exclude = [
912
"go.mod",

LICENSES/CC0-1.0.txt

Lines changed: 0 additions & 121 deletions
This file was deleted.

cloudprofilesync/imageupdater.go

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,33 @@ import (
1717
func filterImages(log logr.Logger, versions []SourceImage) []SourceImage {
1818
filtered := make([]SourceImage, 0, len(versions))
1919
for _, version := range versions {
20-
versionStr := version.effectiveVersion()
21-
_, err := semver.Parse(versionStr)
22-
if err != nil {
23-
log.V(1).Info("skipping invalid version", "version", versionStr)
20+
if len(version.Architectures) == 0 {
21+
log.V(1).Info("skipping version with no architectures", "version", version.Version)
2422
continue
2523
}
26-
if len(version.Architectures) == 0 {
27-
log.V(1).Info("skipping version with no architectures", "version", versionStr)
24+
25+
validLegacyTag := false
26+
if _, err := semver.Parse(version.Version); err == nil {
27+
validLegacyTag = true
28+
}
29+
30+
validCleanVersion := false
31+
if version.CleanVersion != "" {
32+
// Found that we can have "1921.0" in annotations. It will be transformed to "1921.0.0"
33+
if parsed, err := semver.ParseTolerant(version.CleanVersion); err == nil {
34+
validCleanVersion = true
35+
version.CleanVersion = parsed.String()
36+
} else {
37+
log.V(1).Info("ignoring invalid clean version annotation", "tag", version.Version, "cleanVersion", version.CleanVersion)
38+
version.CleanVersion = ""
39+
}
40+
}
41+
42+
if !validLegacyTag && !validCleanVersion {
43+
log.V(1).Info("skipping invalid version (both tag and clean version are bad)", "tag", version.Version)
2844
continue
2945
}
46+
3047
filtered = append(filtered, version)
3148
}
3249
return filtered
@@ -73,13 +90,21 @@ func (iu *ImageUpdater) Update(ctx context.Context, cpSpec *gardenerv1beta1.Clou
7390
if idx, exists := existingVersions[sourceImage.Version]; exists {
7491
image.Versions[idx].Architectures = sourceImage.Architectures
7592
} else {
76-
image.Versions = append(image.Versions, gardenerv1beta1.MachineImageVersion{
77-
ExpirableVersion: gardenerv1beta1.ExpirableVersion{
78-
Version: sourceImage.Version,
79-
},
80-
Architectures: sourceImage.Architectures,
81-
})
82-
existingVersions[sourceImage.Version] = len(image.Versions) - 1
93+
// Moving this check to filterImages() would break the core architectural goal of GEP-33
94+
// as it intentionally decouples the OCI registry tag from the semantic OS version
95+
// In the future, teams might push images with tags like build-0849f313 or 2026-06-release
96+
// As long as the CleanVersion annotation is a valid SemVer (e.g., 2262.0.0), the extension needs to route to it
97+
if _, err = semver.Parse(sourceImage.Version); err != nil {
98+
iu.Log.V(1).Info("skipping legacy entry in spec.machineImages because original tag is not valid semver", "version", sourceImage.Version)
99+
} else {
100+
image.Versions = append(image.Versions, gardenerv1beta1.MachineImageVersion{
101+
ExpirableVersion: gardenerv1beta1.ExpirableVersion{
102+
Version: sourceImage.Version,
103+
},
104+
Architectures: sourceImage.Architectures,
105+
})
106+
existingVersions[sourceImage.Version] = len(image.Versions) - 1
107+
}
83108
}
84109

85110
// When capabilities are enabled, also write the clean version entry.

cloudprofilesync/imageupdater_test.go

Lines changed: 132 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,108 @@ import (
1414
"github.com/cobaltcore-dev/cloud-profile-sync/cloudprofilesync"
1515
)
1616

17-
var _ = Describe("ImageUpdater", func() {
17+
var _ = Describe("filterImages", func() {
18+
// helper: run Update and return the versions written to spec.machineImages
19+
versions := func(ctx SpecContext, images []cloudprofilesync.SourceImage) []gardencorev1beta1.MachineImageVersion {
20+
mockSource.images = images
21+
updater := cloudprofilesync.ImageUpdater{
22+
Log: GinkgoLogr,
23+
Source: &mockSource,
24+
ImageName: "test",
25+
EnableCapabilities: true,
26+
}
27+
var cpSpec gardencorev1beta1.CloudProfileSpec
28+
Expect(updater.Update(ctx, &cpSpec)).To(Succeed())
29+
if len(cpSpec.MachineImages) == 0 {
30+
return nil
31+
}
32+
return cpSpec.MachineImages[0].Versions
33+
}
34+
35+
It("invalid tag + no clean version: drops the image entirely", func(ctx SpecContext) {
36+
result := versions(ctx, []cloudprofilesync.SourceImage{
37+
{Version: "not-a-version", Architectures: []string{"amd64"}},
38+
})
39+
Expect(result).To(BeEmpty())
40+
})
41+
42+
It("invalid tag + invalid clean version: drops the image entirely", func(ctx SpecContext) {
43+
result := versions(ctx, []cloudprofilesync.SourceImage{
44+
{Version: "not-a-version", CleanVersion: "also-not-a-version", Architectures: []string{"amd64"}},
45+
})
46+
Expect(result).To(BeEmpty())
47+
})
48+
49+
It("invalid tag + valid clean version: NEW format only (no legacy entry)", func(ctx SpecContext) {
50+
result := versions(ctx, []cloudprofilesync.SourceImage{
51+
{
52+
Version: "1877.9.2.0-metal-sci-pxe-amd64",
53+
CleanVersion: "1877.9.2",
54+
Architectures: []string{"amd64"},
55+
Capabilities: gardencorev1beta1.Capabilities{"architecture": {"amd64"}, "feature": {"sci", "_pxe"}},
56+
},
57+
})
58+
Expect(result).To(HaveLen(1))
59+
Expect(result[0].Version).To(Equal("1877.9.2"))
60+
})
61+
62+
It("valid tag + valid clean version: BOTH formats", func(ctx SpecContext) {
63+
result := versions(ctx, []cloudprofilesync.SourceImage{
64+
{
65+
Version: "2254.0.0-baremetal-sci-usi-amd64",
66+
CleanVersion: "2254.0.0",
67+
Architectures: []string{"amd64"},
68+
Capabilities: gardencorev1beta1.Capabilities{"architecture": {"amd64"}, "feature": {"sci", "_usi"}},
69+
},
70+
})
71+
Expect(result).To(HaveLen(2))
72+
versionStrings := []string{result[0].Version, result[1].Version}
73+
Expect(versionStrings).To(ContainElements("2254.0.0-baremetal-sci-usi-amd64", "2254.0.0"))
74+
})
75+
76+
It("valid tag + no clean version: OLD format only", func(ctx SpecContext) {
77+
result := versions(ctx, []cloudprofilesync.SourceImage{
78+
{Version: "1921.0.0", Architectures: []string{"amd64"}},
79+
})
80+
Expect(result).To(HaveLen(1))
81+
Expect(result[0].Version).To(Equal("1921.0.0"))
82+
})
83+
84+
It("valid tag + invalid clean version: BOTH formats with clean version normalized", func(ctx SpecContext) {
85+
result := versions(ctx, []cloudprofilesync.SourceImage{
86+
{
87+
Version: "1921.0.0-metal-sci-usi-amd64",
88+
CleanVersion: "1921.0",
89+
Architectures: []string{"amd64"},
90+
Capabilities: gardencorev1beta1.Capabilities{"architecture": {"amd64"}, "feature": {"sci", "_usi"}},
91+
},
92+
})
93+
Expect(result).To(HaveLen(2))
94+
versionStrings := []string{result[0].Version, result[1].Version}
95+
Expect(versionStrings).To(ContainElements("1921.0.0-metal-sci-usi-amd64", "1921.0.0"))
96+
})
97+
98+
It("valid tag + unparsable clean version: does not write clean version entry", func(ctx SpecContext) {
99+
result := versions(ctx, []cloudprofilesync.SourceImage{
100+
{
101+
Version: "1921.0.0-metal-sci-usi-amd64",
102+
CleanVersion: "not-a-version",
103+
Architectures: []string{"amd64"},
104+
},
105+
})
106+
Expect(result).To(HaveLen(1))
107+
Expect(result[0].Version).To(Equal("1921.0.0-metal-sci-usi-amd64"))
108+
})
18109

110+
It("no architectures: drops the image entirely", func(ctx SpecContext) {
111+
result := versions(ctx, []cloudprofilesync.SourceImage{
112+
{Version: "1.0.0"},
113+
})
114+
Expect(result).To(BeEmpty())
115+
})
116+
})
117+
118+
var _ = Describe("ImageUpdater", func() {
19119
Describe("flag OFF (default behavior)", func() {
20120
It("adds an image from the source to the CloudProfile spec", func(ctx SpecContext) {
21121
mockSource.images = []cloudprofilesync.SourceImage{{Version: "1.0.0", Architectures: []string{"amd64"}}}
@@ -172,6 +272,37 @@ var _ = Describe("ImageUpdater", func() {
172272
Expect(cpSpec.MachineImages[0].Versions).To(HaveLen(2))
173273
})
174274

275+
It("skips legacy spec entry for non-semver raw tag but still passes image to provider", func(ctx SpecContext) {
276+
mockSource.images = []cloudprofilesync.SourceImage{
277+
{
278+
Version: "1877.9.2.0-metal-sci-pxe-amd64-1877-9-2-6bb2b442",
279+
CleanVersion: "1877.9.2",
280+
Architectures: []string{"amd64"},
281+
Capabilities: gardencorev1beta1.Capabilities{"architecture": {"amd64"}, "feature": {"sci", "_pxe"}},
282+
},
283+
}
284+
updater := cloudprofilesync.ImageUpdater{
285+
Log: GinkgoLogr,
286+
Source: &mockSource,
287+
ImageName: "test",
288+
EnableCapabilities: true,
289+
Provider: &MockProvider{},
290+
}
291+
var cpSpec gardencorev1beta1.CloudProfileSpec
292+
Expect(updater.Update(ctx, &cpSpec)).To(Succeed())
293+
294+
// Non-semver raw tag must not appear in spec.machineImages — Gardener would reject it.
295+
// Only the clean version entry should be written.
296+
Expect(cpSpec.MachineImages[0].Versions).To(HaveLen(1))
297+
Expect(cpSpec.MachineImages[0].Versions[0].Version).To(Equal("1877.9.2"))
298+
299+
// The raw tag must still reach the provider (capabilityFlavors).
300+
var fromProvider []cloudprofilesync.SourceImage
301+
Expect(json.Unmarshal(cpSpec.ProviderConfig.Raw, &fromProvider)).To(Succeed())
302+
Expect(fromProvider).To(HaveLen(1))
303+
Expect(fromProvider[0].Version).To(Equal("1877.9.2.0-metal-sci-pxe-amd64-1877-9-2-6bb2b442"))
304+
})
305+
175306
It("writes only full tag when CleanVersion is absent", func(ctx SpecContext) {
176307
mockSource.images = []cloudprofilesync.SourceImage{
177308
{Version: "1877.0.0", Architectures: []string{"amd64"}},

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/cobaltcore-dev/cloud-profile-sync
22

3-
go 1.26
3+
go 1.26.2
44

55
require (
66
github.com/blang/semver/v4 v4.0.0

0 commit comments

Comments
 (0)