Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit202e6a0

Browse files
authored
fix: add validation to spec.helm.chart (GoogleContainerTools#1744)
This change adds validation to spec.helm.chart to ensure that it contains no slashes. A KNV1061 error is returned to the user if this condition is not met.E2E tests validate that this error is returned for both charts hosted in OCI-based registries and traditional Helm reposb/423072233
1 parent6cc2b0b commit202e6a0

File tree

5 files changed

+110
-7
lines changed

5 files changed

+110
-7
lines changed

‎e2e/testcases/helm_sync_test.go‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"errors"
2020
"fmt"
2121
"os"
22+
"path"
2223
"path/filepath"
2324
"strconv"
2425
"strings"
@@ -135,6 +136,47 @@ func TestPublicHelm(t *testing.T) {
135136
nt.Must(nt.Watcher.WatchForCurrentStatus(kinds.Deployment(),"my-wordpress",configsync.DefaultHelmReleaseNamespace))
136137
nt.Must(nt.Watcher.WatchForNotFound(kinds.Deployment(),"my-wordpress","wordpress"))
137138
nt.Must(nt.Watcher.WatchForNotFound(kinds.Deployment(),"my-wordpress","deploy-ns"))
139+
140+
nt.T.Log("Update RootSync to sync from a public Helm chart of the form <repo-name>/<chart-name>")
141+
repoSuffix:=path.Base(rs.Spec.Helm.Repo)
142+
chart:=fmt.Sprintf("%s/%s",repoSuffix,rs.Spec.Helm.Chart)
143+
repo:=strings.TrimSuffix(rs.Spec.Helm.Repo,"/"+repoSuffix)
144+
nt.MustMergePatch(rs,fmt.Sprintf(`{"spec": {"helm": {"chart": "%s", "repo": "%s"}}}`,chart,repo))
145+
nt.Must(nt.Watcher.WatchForRootSyncStalledError(rs.Name,"Validation",
146+
`KNV1061: RootSync field spec.helm.chart must not contain a slash (/)`))
147+
}
148+
149+
// TestOCIHelmChartNameContainsSlash verifies the Config Sync behavior for OCI-based Helm charts when helm.spec.chart is of the form <repo-name>/<chart-name>
150+
// This test will work only with following pre-requisite:
151+
// Google service account `e2e-test-ar-reader@${GCP_PROJECT}.iam.gserviceaccount.com` is created with `roles/artifactregistry.reader` for accessing images in Artifact Registry.
152+
funcTestOCIHelmChartNameContainsSlash(t*testing.T) {
153+
rootSyncID:=nomostest.DefaultRootSyncID
154+
nt:=nomostest.New(t,
155+
nomostesting.SyncSource,
156+
ntopts.SyncWithGitSource(nomostest.DefaultRootSyncID,ntopts.Unstructured),
157+
ntopts.RequireHelmProvider,
158+
)
159+
160+
newVersion:="1.0.0"
161+
chart,err:=nt.BuildAndPushHelmPackage(rootSyncID.ObjectKey,
162+
registryproviders.HelmSourceChart(privateSimpleHelmChart),
163+
registryproviders.HelmChartVersion(newVersion))
164+
iferr!=nil {
165+
nt.T.Fatalf("failed to push helm chart: %v",err)
166+
}
167+
168+
rs:=nt.RootSyncObjectHelm(configsync.RootSyncName,chart.HelmChartID)
169+
rs.Spec.Helm.Version=""
170+
rs.Spec.Helm.DeployNamespace="simple"
171+
172+
repoSuffix:=path.Base(rs.Spec.Helm.Repo)
173+
rs.Spec.Helm.Chart=fmt.Sprintf("%s/%s",repoSuffix,rs.Spec.Helm.Chart)
174+
rs.Spec.Helm.Repo=strings.TrimSuffix(rs.Spec.Helm.Repo,"/"+repoSuffix)
175+
176+
nt.T.Log("Update RootSync to sync from a helm chart")
177+
nt.Must(nt.KubeClient.Apply(rs))
178+
nt.Must(nt.Watcher.WatchForRootSyncStalledError(rs.Name,"Validation",
179+
`KNV1061: RootSync field spec.helm.chart must not contain a slash (/)`))
138180
}
139181

140182
// TestHelmWatchConfigMap can run on both Kind and GKE clusters.

‎pkg/reconcilermanager/controllers/reposync_controller_test.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3775,6 +3775,9 @@ func TestRepoSyncWithOCI(t *testing.T) {
37753775
}
37763776

37773777
funcTestRepoSyncSpecValidation(t*testing.T) {
3778+
// Mock out parseDeployment for testing.
3779+
parseDeployment=parsedDeployment
3780+
37783781
rs:=k8sobjects.RepoSyncObjectV1Beta1(reposyncNs,reposyncName)
37793782
reqNamespacedName:=namespacedName(rs.Name,rs.Namespace)
37803783
fakeClient,_,testReconciler:=setupNSReconciler(t,rs)
@@ -3914,6 +3917,24 @@ func TestRepoSyncSpecValidation(t *testing.T) {
39143917
reposync.SetStalled(wantRs,"Validation",validate.InvalidHelmAuthType(configsync.RepoSyncKind))
39153918
validateRepoSyncStatus(t,wantRs,fakeClient)
39163919

3920+
// verify invalid Helm chart name
3921+
iferr:=fakeClient.Get(ctx,client.ObjectKeyFromObject(rs),rs);err!=nil {
3922+
t.Fatalf("failed to get the repo sync: %v",err)
3923+
}
3924+
rs.Spec.SourceType=configsync.HelmSource
3925+
rs.Spec.Git=nil
3926+
rs.Spec.Oci=nil
3927+
rs.Spec.Helm=&v1beta1.HelmRepoSync{HelmBase: v1beta1.HelmBase{Repo:helmRepo,Chart:"test/"+helmChart,Auth:configsync.AuthNone}}
3928+
iferr:=fakeClient.Update(ctx,rs,client.FieldOwner(reconcilermanager.FieldManager));err!=nil {
3929+
t.Fatalf("failed to update the repo sync request, got error: %v",err)
3930+
}
3931+
if_,err:=testReconciler.Reconcile(ctx,reqNamespacedName);err!=nil {
3932+
t.Fatalf("unexpected reconciliation error, got error: %q, want error: nil",err)
3933+
}
3934+
wantRs.Spec=rs.Spec
3935+
reposync.SetStalled(wantRs,"Validation",validate.IllegalHelmChartName(configsync.RepoSyncKind))
3936+
validateRepoSyncStatus(t,wantRs,fakeClient)
3937+
39173938
// verify valid OCI spec
39183939
iferr:=fakeClient.Get(ctx,client.ObjectKeyFromObject(rs),rs);err!=nil {
39193940
t.Fatalf("failed to get the repo sync: %v",err)

‎pkg/reconcilermanager/controllers/rootsync_controller_test.go‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3788,6 +3788,23 @@ func TestRootSyncSpecValidation(t *testing.T) {
37883788
rootsync.SetStalled(wantRs,"Validation",validate.InvalidHelmAuthType(configsync.RootSyncKind))
37893789
validateRootSyncStatus(t,wantRs,fakeClient)
37903790

3791+
// verify invalid Helm chart name
3792+
iferr:=fakeClient.Get(ctx,client.ObjectKeyFromObject(rs),rs);err!=nil {
3793+
t.Fatalf("failed to get the root sync: %v",err)
3794+
}
3795+
invalidHelmChart:=fmt.Sprintf("%s/",helmChart)
3796+
rs.Spec.SourceType=configsync.HelmSource
3797+
rs.Spec.Helm=&v1beta1.HelmRootSync{HelmBase: v1beta1.HelmBase{Repo:helmRepo,Chart:invalidHelmChart}}
3798+
iferr:=fakeClient.Update(ctx,rs,client.FieldOwner(reconcilermanager.FieldManager));err!=nil {
3799+
t.Fatalf("failed to update the root sync request, got error: %v",err)
3800+
}
3801+
if_,err:=testReconciler.Reconcile(ctx,reqNamespacedName);err!=nil {
3802+
t.Fatalf("unexpected reconciliation error, got error: %q, want error: nil",err)
3803+
}
3804+
wantRs.Spec=rs.Spec
3805+
rootsync.SetStalled(wantRs,"Validation",validate.IllegalHelmChartName(configsync.RootSyncKind))
3806+
validateRootSyncStatus(t,wantRs,fakeClient)
3807+
37913808
// verify valid OCI spec
37923809
iferr:=fakeClient.Get(ctx,client.ObjectKeyFromObject(rs),rs);err!=nil {
37933810
t.Fatalf("failed to get the root sync: %v",err)

‎pkg/validate/rsync/validate/source_spec_validator.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ func HelmSpec(helm *v1beta1.HelmBase, syncKind string) status.Error {
199199
returnMissingHelmChart(syncKind)
200200
}
201201

202+
// Validates chart name such that it contains no slashes
203+
// See https://helm.sh/docs/chart_best_practices/conventions/ for more details
204+
ifstrings.Contains(helm.Chart,"/") {
205+
returnIllegalHelmChartName(syncKind)
206+
}
207+
202208
// Ensure auth is a valid value.
203209
// Note that Auth is a case-sensitive field, so ones with arbitrary capitalization
204210
// will fail to apply.
@@ -392,6 +398,13 @@ func NoOpProxy(syncKind string) status.Error {
392398
Build()
393399
}
394400

401+
// IllegalHelmChartName reports that a RootSync/RepoSync declares an invalid helm chart name.
402+
funcIllegalHelmChartName(syncKindstring) status.Error {
403+
returninvalidSyncBuilder.
404+
Sprintf("%s field spec.helm.chart must not contain a slash (/)",syncKind).
405+
Build()
406+
}
407+
395408
// IllegalSecretRef reports that a RootSync/RepoSync declares an auth mode that doesn't
396409
// allow SecretRefs does declare a SecretRef.
397410
funcIllegalSecretRef(sourceType configsync.SourceType,syncKindstring) status.Error {

‎pkg/validate/rsync/validate/source_spec_validator_test.go‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ func repoSyncWithGit(opts ...func(*v1beta1.RepoSync)) *v1beta1.RepoSync {
8989
rs:=k8sobjects.RepoSyncObjectV1Beta1("test-ns",configsync.RepoSyncName)
9090
rs.Spec.SourceType=configsync.GitSource
9191
rs.Spec.Git=&v1beta1.Git{
92-
Repo:"fakerepo",
92+
Repo:"fake-repo",
9393
Auth:configsync.AuthNone,
9494
}
9595
for_,opt:=rangeopts {
@@ -102,7 +102,7 @@ func repoSyncWithOci(opts ...func(*v1beta1.RepoSync)) *v1beta1.RepoSync {
102102
rs:=k8sobjects.RepoSyncObjectV1Beta1("test-ns",configsync.RepoSyncName)
103103
rs.Spec.SourceType=configsync.OciSource
104104
rs.Spec.Oci=&v1beta1.Oci{
105-
Image:"fakeimage",
105+
Image:"fake-image",
106106
Auth:configsync.AuthNone,
107107
}
108108
for_,opt:=rangeopts {
@@ -115,8 +115,8 @@ func repoSyncWithHelm(opts ...func(*v1beta1.RepoSync)) *v1beta1.RepoSync {
115115
rs:=k8sobjects.RepoSyncObjectV1Beta1("test-ns",configsync.RepoSyncName)
116116
rs.Spec.SourceType=configsync.HelmSource
117117
rs.Spec.Helm=&v1beta1.HelmRepoSync{HelmBase: v1beta1.HelmBase{
118-
Repo:"fakerepo",
119-
Chart:"fakechart",
118+
Repo:"fake-repo",
119+
Chart:"fake-chart",
120120
Auth:configsync.AuthNone,
121121
}}
122122
for_,opt:=rangeopts {
@@ -155,7 +155,7 @@ func rootSyncWithGit(opts ...func(*v1beta1.RootSync)) *v1beta1.RootSync {
155155
rs:=k8sobjects.RootSyncObjectV1Beta1(configsync.RootSyncName)
156156
rs.Spec.SourceType=configsync.GitSource
157157
rs.Spec.Git=&v1beta1.Git{
158-
Repo:"fakerepo",
158+
Repo:"fake-repo",
159159
Auth:configsync.AuthNone,
160160
}
161161
for_,opt:=rangeopts {
@@ -168,8 +168,8 @@ func rootSyncWithHelm(opts ...func(*v1beta1.RootSync)) *v1beta1.RootSync {
168168
rs:=k8sobjects.RootSyncObjectV1Beta1(configsync.RootSyncName)
169169
rs.Spec.SourceType=configsync.HelmSource
170170
rs.Spec.Helm=&v1beta1.HelmRootSync{HelmBase: v1beta1.HelmBase{
171-
Repo:"fakerepo",
172-
Chart:"fakechart",
171+
Repo:"fake-repo",
172+
Chart:"fake-chart",
173173
Auth:configsync.AuthNone,
174174
}}
175175
for_,opt:=rangeopts {
@@ -300,6 +300,11 @@ func TestValidateRepoSyncSpec(t *testing.T) {
300300
obj:repoSyncWithHelm(missingHelmChart),
301301
wantErr:MissingHelmChart(configsync.RepoSyncKind),
302302
},
303+
{
304+
name:"illegal helm chart name with slash",
305+
obj:repoSyncWithHelm(func(rs*v1beta1.RepoSync) {rs.Spec.Helm.Chart="foo/bar" }),
306+
wantErr:IllegalHelmChartName(configsync.RepoSyncKind),
307+
},
303308
{
304309
name:"invalid auth type",
305310
obj:repoSyncWithHelm(helmAuth("invalid auth")),
@@ -418,6 +423,11 @@ func TestValidateRootSyncSpec(t *testing.T) {
418423
}),
419424
wantErr:HelmNSAndDeployNS(configsync.RootSyncKind),
420425
},
426+
{
427+
name:"invalid helm chart name with slash",
428+
obj:rootSyncWithHelm(func(rs*v1beta1.RootSync) {rs.Spec.Helm.Chart="foo/bar" }),
429+
wantErr:IllegalHelmChartName(configsync.RootSyncKind),
430+
},
421431
{
422432
name:"valid spec.override.roleRefs Role",
423433
obj:rootSyncWithGit(func(sync*v1beta1.RootSync) {

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp