Skip to content

Commit 3d45be3

Browse files
AYDEV-FRVad1moCopilot
authored
fix: proxy cache serve local on remote not found (#22153)
* fix: proxy cache serve local on remote not found Signed-off-by: AYDEV-FR <aymeric.deliencourt@aydev.fr> * tests: fix proxy controller test Signed-off-by: AYDEV-FR <aymeric.deliencourt@aydev.fr> * Update src/controller/proxy/controller.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Vadim Bauer <Bauer.vadim@gmail.com> * feat: add ui Signed-off-by: AYDEV-FR <aymeric.deliencourt@aydev.fr> * chore: add checkbox in creation project modal Signed-off-by: AYDEV-FR <aymeric.deliencourt@aydev.fr> * chore: fix tooltip text Signed-off-by: AYDEV-FR <aymeric.deliencourt@aydev.fr> --------- Signed-off-by: AYDEV-FR <aymeric.deliencourt@aydev.fr> Signed-off-by: Vadim Bauer <Bauer.vadim@gmail.com> Co-authored-by: Vadim Bauer <Bauer.vadim@gmail.com> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 0af2484 commit 3d45be3

File tree

16 files changed

+134
-30
lines changed

16 files changed

+134
-30
lines changed

api/v2.0/swagger.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7336,6 +7336,10 @@ definitions:
73367336
type: string
73377337
description: 'The max connection per artifact to the upstream registry in current proxy cache project, if it is -1, no limit to upstream registry connections'
73387338
x-nullable: true
7339+
proxy_cache_local_on_not_found:
7340+
type: string
7341+
description: 'Whether to serve images from local cache when they are removed from the upstream registry. The valid values are "true", "false".'
7342+
x-nullable: true
73397343
ProjectSummary:
73407344
type: object
73417345
properties:

src/controller/proxy/controller.go

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ type Controller interface {
5858
// UseLocalBlob check if the blob should use local copy
5959
UseLocalBlob(ctx context.Context, art lib.ArtifactInfo) bool
6060
// UseLocalManifest check manifest should use local copy
61-
UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *ManifestList, error)
61+
UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface, p *proModels.Project) (bool, *ManifestList, error)
6262
// ProxyBlob proxy the blob request to the remote server, p is the proxy project
6363
// art is the ArtifactInfo which includes the digest of the blob
6464
ProxyBlob(ctx context.Context, p *proModels.Project, art lib.ArtifactInfo) (int64, io.ReadCloser, error)
@@ -169,7 +169,7 @@ func (c *controller) getManifestDigestInLocal(ctx context.Context, art lib.Artif
169169
// the return error should be nil when it is not found in local and need to delegate to remote registry
170170
// the return error should be NotFoundError when it is not found in remote registry
171171
// the error will be captured by framework and return 404 to client
172-
func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface) (bool, *ManifestList, error) {
172+
func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo, remote RemoteInterface, p *proModels.Project) (bool, *ManifestList, error) {
173173
a, err := c.local.GetManifest(ctx, art)
174174
if err != nil {
175175
return false, nil, err
@@ -188,15 +188,10 @@ func (c *controller) UseLocalManifest(ctx context.Context, art lib.ArtifactInfo,
188188
return false, nil, err
189189
}
190190
if !exist || desc == nil {
191-
dig, err := c.getManifestDigestInLocal(ctx, art)
192-
if err != nil {
193-
// skip to delete when error, use debug level log to avoid too many logs when the manifest is removed from upstream
194-
log.Debugf("failed to get manifest digest in local, error: %v, skip to delete it, art %+v", err, art)
195-
} else {
196-
go func() {
197-
c.local.DeleteManifest(art.Repository, dig)
198-
log.Infof("delete manifest %s with digest %s", art.Repository, dig)
199-
}()
191+
// if not found in remote and local artifact exists, check if we should serve from local cache
192+
if a != nil && p != nil && p.ProxyCacheLocalOnNotFound() {
193+
log.Infof("Artifact not found in remote registry but exists in local cache, serving from local: %v:%v", art.Repository, art.Tag)
194+
return true, nil, nil
200195
}
201196
return false, nil, errors.NotFoundError(fmt.Errorf("repo %v, tag %v not found", art.Repository, art.Tag))
202197
}

src/controller/proxy/controller_test.go

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_True() {
106106
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
107107
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
108108

109-
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
109+
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote, p.proj)
110110
p.Assert().Nil(err)
111111
p.Assert().True(result)
112112
}
@@ -118,7 +118,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_False() {
118118
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
119119
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(true, desc, nil)
120120
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil)
121-
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
121+
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote, p.proj)
122122
p.Assert().Nil(err)
123123
p.Assert().False(result)
124124
}
@@ -130,7 +130,7 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_429() {
130130
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
131131
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, errors.New("too many requests").WithCode(errors.RateLimitCode))
132132
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(nil, nil)
133-
_, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
133+
_, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote, p.proj)
134134
p.Assert().NotNil(err)
135135
errors.IsRateLimitError(err)
136136
}
@@ -142,19 +142,40 @@ func (p *proxyControllerTestSuite) TestUseLocalManifest_429ToLocal() {
142142
art := lib.ArtifactInfo{Repository: "library/hello-world", Digest: dig}
143143
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, errors.New("too many requests").WithCode(errors.RateLimitCode))
144144
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
145-
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
145+
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote, p.proj)
146146
p.Assert().Nil(err)
147147
p.Assert().True(result)
148148
}
149149

150-
func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_False() {
150+
func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_True() {
151151
ctx := context.Background()
152152
art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"}
153153
desc := &distribution.Descriptor{}
154+
// Set up project with ProxyCacheLocalOnNotFound enabled
155+
proj := &proModels.Project{
156+
RegistryID: 1,
157+
Metadata: map[string]string{proModels.ProMetaProxyCacheLocalOnNotFound: "true"},
158+
}
159+
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
160+
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, nil)
161+
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote, proj)
162+
p.Assert().Nil(err)
163+
p.Assert().True(result)
164+
}
165+
166+
func (p *proxyControllerTestSuite) TestUseLocalManifestWithTag_NotFoundWhenOptionDisabled() {
167+
ctx := context.Background()
168+
art := lib.ArtifactInfo{Repository: "library/hello-world", Tag: "latest"}
169+
desc := &distribution.Descriptor{}
170+
// Set up project without ProxyCacheLocalOnNotFound enabled
171+
proj := &proModels.Project{
172+
RegistryID: 1,
173+
Metadata: map[string]string{},
174+
}
154175
p.local.On("GetManifest", mock.Anything, mock.Anything).Return(&artifact.Artifact{}, nil)
155176
p.remote.On("ManifestExist", mock.Anything, mock.Anything).Return(false, desc, nil)
156-
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote)
157-
p.Assert().True(errors.IsNotFoundErr(err))
177+
result, _, err := p.ctr.UseLocalManifest(ctx, art, p.remote, proj)
178+
p.Assert().NotNil(err)
158179
p.Assert().False(result)
159180
}
160181

src/pkg/project/models/pro_meta.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,15 @@ package models
1616

1717
// keys of project metadata and severity values
1818
const (
19-
ProMetaPublic = "public"
20-
ProMetaEnableContentTrust = "enable_content_trust"
21-
ProMetaEnableContentTrustCosign = "enable_content_trust_cosign"
22-
ProMetaPreventVul = "prevent_vul" // prevent vulnerable images from being pulled
23-
ProMetaSeverity = "severity"
24-
ProMetaAutoScan = "auto_scan"
25-
ProMetaReuseSysCVEAllowlist = "reuse_sys_cve_allowlist"
26-
ProMetaAutoSBOMGen = "auto_sbom_generation"
27-
ProMetaProxySpeed = "proxy_speed_kb"
28-
ProMetaMaxUpstreamConn = "max_upstream_conn"
19+
ProMetaPublic = "public"
20+
ProMetaEnableContentTrust = "enable_content_trust"
21+
ProMetaEnableContentTrustCosign = "enable_content_trust_cosign"
22+
ProMetaPreventVul = "prevent_vul" // prevent vulnerable images from being pulled
23+
ProMetaSeverity = "severity"
24+
ProMetaAutoScan = "auto_scan"
25+
ProMetaReuseSysCVEAllowlist = "reuse_sys_cve_allowlist"
26+
ProMetaAutoSBOMGen = "auto_sbom_generation"
27+
ProMetaProxySpeed = "proxy_speed_kb"
28+
ProMetaMaxUpstreamConn = "max_upstream_conn"
29+
ProMetaProxyCacheLocalOnNotFound = "proxy_cache_local_on_not_found"
2930
)

src/pkg/project/models/project.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,15 @@ func (p *Project) MaxUpstreamConnection() int {
184184
return int(cnt)
185185
}
186186

187+
// ProxyCacheLocalOnNotFound returns true if images should be served from local cache when removed from upstream
188+
func (p *Project) ProxyCacheLocalOnNotFound() bool {
189+
val, exist := p.GetMetadata(ProMetaProxyCacheLocalOnNotFound)
190+
if !exist {
191+
return false
192+
}
193+
return isTrue(val)
194+
}
195+
187196
// FilterByPublic returns orm.QuerySeter with public filter
188197
func (p *Project) FilterByPublic(_ context.Context, qs orm.QuerySeter, _ string, value any) orm.QuerySeter {
189198
subQuery := `SELECT project_id FROM project_metadata WHERE name = 'public' AND value = '%s'`

src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.html

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,39 @@ <h3 class="modal-title">{{ 'PROJECT.NEW_PROJECT' | translate }}</h3>
330330
</clr-control-error>
331331
</div>
332332
</div>
333+
<clr-checkbox-container
334+
[style.visibility]="
335+
isSystemAdmin && enableProxyCache ? 'visible' : 'hidden'
336+
">
337+
<label class="clr-control-label">
338+
{{ 'PROJECT.PROXY_CACHE_LOCAL_ON_NOT_FOUND' | translate }}
339+
<clr-tooltip>
340+
<clr-icon
341+
clrTooltipTrigger
342+
shape="info-circle"
343+
size="24"></clr-icon>
344+
<clr-tooltip-content
345+
clrPosition="bottom-left"
346+
clrSize="lg"
347+
*clrIfOpen>
348+
<span>{{
349+
'PROJECT.PROXY_CACHE_LOCAL_ON_NOT_FOUND_TOOLTIP'
350+
| translate
351+
}}</span>
352+
</clr-tooltip-content>
353+
</clr-tooltip>
354+
</label>
355+
<clr-checkbox-wrapper>
356+
<input
357+
clrCheckbox
358+
type="checkbox"
359+
id="proxy_cache_local_on_not_found"
360+
[(ngModel)]="
361+
project.metadata.proxy_cache_local_on_not_found
362+
"
363+
name="proxy_cache_local_on_not_found" />
364+
</clr-checkbox-wrapper>
365+
</clr-checkbox-container>
333366
</form>
334367
</div>
335368
<div class="modal-footer">

src/portal/src/app/base/left-side-nav/projects/create-project/create-project.component.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,10 @@ export class CreateProjectComponent
395395
this.project.metadata.bandwidth.toString(),
396396
max_upstream_conn:
397397
this.project.metadata.max_upstream_conn.toString(),
398+
proxy_cache_local_on_not_found: this.project.metadata
399+
.proxy_cache_local_on_not_found
400+
? 'true'
401+
: 'false',
398402
},
399403
storage_limit: +storageByte,
400404
registry_id: registryId,
@@ -440,6 +444,7 @@ export class CreateProjectComponent
440444
this.selectedSpeedLimitUnit = BandwidthUnit.KB;
441445
this.speedLimit = -1;
442446
this.project.metadata.max_upstream_conn = -1;
447+
this.project.metadata.proxy_cache_local_on_not_found = false;
443448
}
444449

445450
public get isValid(): boolean {

src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.html

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,27 @@
122122
</div>
123123
</div>
124124
</div>
125+
<clr-checkbox-container
126+
*ngIf="isSystemAdmin && projectPolicy.ProxyCacheEnabled">
127+
<label class="clr-control-label"></label>
128+
<clr-checkbox-wrapper id="proxy-cache-local-on-not-found">
129+
<input
130+
type="checkbox"
131+
id="proxy-cache-local-on-not-found-input"
132+
clrCheckbox
133+
[(ngModel)]="projectPolicy.ProxyCacheLocalOnNotFound"
134+
name="proxy-cache-local-on-not-found"
135+
[disabled]="!hasChangeConfigRole" />
136+
<label for="proxy-cache-local-on-not-found-input">
137+
{{ 'PROJECT.PROXY_CACHE_LOCAL_ON_NOT_FOUND' | translate }}
138+
</label>
139+
</clr-checkbox-wrapper>
140+
<clr-control-helper class="config-subtext">
141+
{{
142+
'PROJECT.PROXY_CACHE_LOCAL_ON_NOT_FOUND_TOOLTIP' | translate
143+
}}
144+
</clr-control-helper>
145+
</clr-checkbox-container>
125146

126147
<clr-checkbox-container *ngIf="!isProxyCacheProject" clrInline>
127148
<label

src/portal/src/app/base/project/project-config/project-policy-config/project-policy-config.component.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export class ProjectPolicy {
5959
RegistryId?: number | null;
6060
ProxySpeedKb?: number | null;
6161
MaxUpstreamConn?: number | null;
62+
ProxyCacheLocalOnNotFound?: boolean;
6263

6364
constructor() {
6465
this.Public = false;
@@ -72,6 +73,7 @@ export class ProjectPolicy {
7273
this.RegistryId = null;
7374
this.ProxySpeedKb = -1;
7475
this.MaxUpstreamConn = -1;
76+
this.ProxyCacheLocalOnNotFound = false;
7577
}
7678

7779
initByProject(pro: Project) {
@@ -93,6 +95,8 @@ export class ProjectPolicy {
9395
this.MaxUpstreamConn = pro.metadata.max_upstream_conn
9496
? pro.metadata.max_upstream_conn
9597
: -1;
98+
this.ProxyCacheLocalOnNotFound =
99+
pro.metadata.proxy_cache_local_on_not_found === 'true';
96100
}
97101
}
98102
const PAGE_SIZE: number = 100;

src/portal/src/app/base/project/project-config/project-policy-config/project.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ export class Project {
3737
reuse_sys_cve_allowlist?: string;
3838
proxy_speed_kb?: number | null;
3939
max_upstream_conn?: number | null;
40+
proxy_cache_local_on_not_found?: string | boolean;
4041
};
4142
cve_allowlist?: object;
4243
constructor() {

0 commit comments

Comments
 (0)