Disable sync periodically polling when pollInterval is not configured

[peusebiu]

Filtering out sync on demand images based on content configuration

Signed-off-by: Petu Eusebiu peusebiu@cisco.com

[peusebiu]

@rchincha
These are two improvements we talked about last week.

Regarding the third one, which is not yet included in this PR, returning UNAUTHORIZED to the client in case a sync on demand gets this error from an upstream registry.

In routes, all the errors returned to the client, are registry specific errors, how can I customize the UNAUTHORIZED error so that I can tell the client which upstream registry throwed that error and that it is not returned by zot.

Can I create a new error /pkg/api/errors.go ?

[codecov]

Codecov Report

Merging #254 (fd5cf5b) into main (c61c383) will increase coverage by 0.07%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #254      +/-   ##
==========================================
+ Coverage   80.75%   80.83%   +0.07%     
==========================================
  Files          46       46              
  Lines        6261     6265       +4     
==========================================
+ Hits         5056     5064       +8     
+ Misses        872      868       -4     
  Partials      333      333              

Impacted Files	Coverage Δ
pkg/extensions/extensions.go	78.18% <ø> (-2.78%)	⬇️
pkg/extensions/sync/http_handler.go	81.25% <100.00%> (+1.93%)	⬆️
pkg/extensions/sync/on_demand.go	75.67% <100.00%> (-8.15%)	⬇️
pkg/extensions/sync/sync.go	88.54% <100.00%> (+0.15%)	⬆️
pkg/cli/root.go	71.89% <0.00%> (+7.18%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c61c383...fd5cf5b. Read the comment docs.

[rchincha]

@rchincha These are two improvements we talked about last week.

Regarding the third one, which is not yet included in this PR, returning UNAUTHORIZED to the client in case a sync on demand gets this error from an upstream registry.

In routes, all the errors returned to the client, are registry specific errors, how can I customize the UNAUTHORIZED error so that I can tell the client which upstream registry throwed that error and that it is not returned by zot.

Can I create a new error /pkg/api/errors.go ?

Why not just wrap the error?

[rchincha]

Thinking about this some more, so if pollInterval is not specified and onDemand is false, then this registry is disabled for sync?

[peusebiu]

Thinking about this some more, so if pollInterval is not specified and onDemand is false, then this registry is disabled for sync?

Yes

[peusebiu]

@rchincha These are two improvements we talked about last week.
Regarding the third one, which is not yet included in this PR, returning UNAUTHORIZED to the client in case a sync on demand gets this error from an upstream registry.
In routes, all the errors returned to the client, are registry specific errors, how can I customize the UNAUTHORIZED error so that I can tell the client which upstream registry throwed that error and that it is not returned by zot.
Can I create a new error /pkg/api/errors.go ?

Why not just wrap the error?

Ok, I wrap the error with the registry name, how can I return it to the client?
In routes we return json objects, but I have only error type objects.

eg:

WriteJSON(w, http.StatusNotFound,
	NewErrorList(NewError(MANIFEST_UNKNOWN, map[string]string{"reference": reference})))

Tried

WriteJSON(w, http.StatusNotFound, map[string]string{"err": err.Error()})

But it doesn't get returned to client.... I don't understand why...

[peusebiu]

I managed to send the error to the client, but if I set the http error code to 500 (InternalServerError) then my sync error doesn't get displayed.
If I use 400 (BadRequest) it doesn't get fully displayed, just a part of it:

FATA[0010] Error initializing source docker://localhost:8081/atomix/alpine:latest: Error reading manifest latest in localhost:8081/atomix/alpine: StatusCode: 400, {"error":"sync:- Error initializing source docker:...

500 case:

FATA[0010] Error initializing source docker://localhost:8081/atomix/alpine:latest: Error reading manifest latest in localhost:8081/atomix/alpine: received unexpected HTTP status: 500 Internal Server Error

[rchincha]

@rchincha These are two improvements we talked about last week.

Regarding the third one, which is not yet included in this PR, returning UNAUTHORIZED to the client in case a sync on demand gets this error from an upstream registry.

In routes, all the errors returned to the client, are registry specific errors, how can I customize the UNAUTHORIZED error so that I can tell the client which upstream registry throwed that error and that it is not returned by zot.

Can I create a new error /pkg/api/errors.go ?

Thinking about this more, if you get Unauthorized from upstream, best to just log it as an error. For the downstream, the image will just not be available, so a NotFound will suffice.

[peusebiu]

@rchincha These are two improvements we talked about last week.
Regarding the third one, which is not yet included in this PR, returning UNAUTHORIZED to the client in case a sync on demand gets this error from an upstream registry.
In routes, all the errors returned to the client, are registry specific errors, how can I customize the UNAUTHORIZED error so that I can tell the client which upstream registry throwed that error and that it is not returned by zot.
Can I create a new error /pkg/api/errors.go ?

Thinking about this more, if you get Unauthorized from upstream, best to just log it as an error. For the downstream, the image will just not be available, so a NotFound will suffice.

That's what I was thinking, it's more an administrator information than a client one. I already log it in sync.

[rchincha]

lgtm