Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(referrers): delete manifest with subject #174

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 30 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
e3e1217
push manifest with subject
Nov 18, 2024
846e173
add unit tests
Nov 19, 2024
7fca0cb
add unit tests
Nov 19, 2024
ec93662
Merge branch 'main' into feature/pushManifestWithSubject
Nov 19, 2024
7809549
resolve merge conflicts
Nov 19, 2024
c7e4418
add tests
Nov 19, 2024
a6d552f
add comments
Nov 19, 2024
685ab70
add unit test
Nov 20, 2024
1b9ade3
add unit tests
Nov 20, 2024
5041592
resolve comments
Nov 21, 2024
c26ccb6
add SetReferrersSupportLevel func and unit tests
Nov 21, 2024
ddbf048
remove NoReferrerUpdateException and update tests accordingly
Nov 22, 2024
d6e8499
add Index constructor
Nov 24, 2024
cf431f0
add license header
Nov 26, 2024
50e696e
resolve merge conflicts
Nov 26, 2024
856c0ef
add lock on SetReferrerState
Nov 29, 2024
fcb121e
simplify ApplyReferrerChanges
Nov 29, 2024
2b24e07
resolve comments
Dec 3, 2024
7df36ca
Merge branch 'main' into feature/pushManifestWithSubject
Dec 3, 2024
36a428f
delete manifest with subject
Dec 10, 2024
1813869
add reference clone
Dec 23, 2024
878535c
change variable naming
Dec 26, 2024
4bee2cc
fix merge conflict
Jan 6, 2025
c848ed2
polish pr
Jan 6, 2025
a7460f3
fix responseException style
Jan 7, 2025
20dcda9
Merge branch 'main' into feature/deleteManifestWithSubject
Jan 13, 2025
d7eb9b5
add unit tests
Jan 13, 2025
dfc538d
remove unnecessary changes
Jan 13, 2025
ff334ca
remove unnecessary changes
Jan 13, 2025
53f8e12
move ResponseException to Exceptions folder
Jan 13, 2025
b13de61
resolve comments
Jan 15, 2025
c5d222a
Merge branch 'main' into feature/deleteManifestWithSubject
Feb 5, 2025
21c714a
move response exception back to remote
Feb 5, 2025
a23d7eb
change to copy constructor
Feb 5, 2025
d5c38b9
dispose resource
Feb 6, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
pat-pan marked this conversation as resolved.
Show resolved Hide resolved
pat-pan marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@
using System.Collections.Generic;
using System.Net;
using System.Net.Http;
using System.Text;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;

namespace OrasProject.Oras.Registry.Remote;

public class ResponseException : HttpRequestException
{
{
public static readonly string ErrorCodeNameUnknown = "NAME_UNKNOWN";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be grouped as an enum for ErrorCode?

public class Error
{
[JsonPropertyName("code")]
Expand All @@ -34,45 +33,45 @@ public class Error

[JsonPropertyName("detail")]
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)]
public JsonElement? Detail { get; set; }
public JsonElement? Detail { get; set; }
}

public class ErrorResponse
{
public class ErrorResponse
{
[JsonPropertyName("errors")]
public required IList<Error> Errors { get; set; }
public required IList<Error> Errors { get; set; }
}

public HttpMethod? Method { get; }

public Uri? RequestUri { get; }

public IList<Error>? Errors { get; }
public ResponseException(HttpResponseMessage response, string? responseBody = null)
: this(response, responseBody, null)
{
}
public ResponseException(HttpResponseMessage response, string? responseBody, string? message)
: this(response, responseBody, response.StatusCode == HttpStatusCode.Unauthorized ? HttpRequestError.UserAuthenticationError : HttpRequestError.Unknown, message, null)
{
}
public ResponseException(HttpResponseMessage response, string? responseBody, HttpRequestError httpRequestError, string? message, Exception? inner)
: base(httpRequestError, message, inner, response.StatusCode)
{
var request = response.RequestMessage;
Method = request?.Method;
RequestUri = request?.RequestUri;
if (responseBody != null)
{
try
{
var errorResponse = JsonSerializer.Deserialize<ErrorResponse>(responseBody);
Errors = errorResponse?.Errors;
}
catch { }
public IList<Error>? Errors { get; }

public ResponseException(HttpResponseMessage response, string? responseBody = null)
: this(response, responseBody, null)
{
}

public ResponseException(HttpResponseMessage response, string? responseBody, string? message)
: this(response, responseBody, response.StatusCode == HttpStatusCode.Unauthorized ? HttpRequestError.UserAuthenticationError : HttpRequestError.Unknown, message, null)
{
}

public ResponseException(HttpResponseMessage response, string? responseBody, HttpRequestError httpRequestError, string? message, Exception? inner)
: base(httpRequestError, message, inner, response.StatusCode)
{
var request = response.RequestMessage;
Method = request?.Method;
RequestUri = request?.RequestUri;
if (responseBody != null)
{
try
{
var errorResponse = JsonSerializer.Deserialize<ErrorResponse>(responseBody);
Errors = errorResponse?.Errors;
}
catch { }
}
}
}
5 changes: 5 additions & 0 deletions src/OrasProject.Oras/Registry/Reference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,11 @@ public static bool TryParse(string reference, [NotNullWhen(true)] out Reference?
}
}

public Reference Clone()
{
return new Reference(Registry, Repository, ContentReference);
}
pat-pan marked this conversation as resolved.
Show resolved Hide resolved

public Reference(string registry) => _registry = ValidateRegistry(registry);

public Reference(string registry, string? repository) : this(registry)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ public static void VerifyContentDigest(this HttpResponseMessage response, string
}
if (contentDigest != expected)
{
throw new HttpIOException(HttpRequestError.InvalidResponse, $"{response.RequestMessage!.Method} {response.RequestMessage.RequestUri}: invalid response; digest mismatch in Docker-Content-Digest: received {contentDigest} when expecting {digestStr}");
throw new HttpIOException(HttpRequestError.InvalidResponse, $"{response.RequestMessage!.Method} {response.RequestMessage.RequestUri}: invalid response; digest mismatch in Docker-Content-Digest: received {contentDigest} when expecting {expected}");
}
}

Expand Down
74 changes: 73 additions & 1 deletion src/OrasProject.Oras/Registry/Remote/ManifestStore.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

public class ManifestStore(Repository repository) : IManifestStore
{
public Repository Repository { get; init; } = repository;

Check warning on line 32 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View workflow job for this annotation

GitHub Actions / Analyze (8.0.x)

Parameter 'Repository repository' is captured into the state of the enclosing type and its value is also used to initialize a field, property, or event.

Check warning on line 32 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View workflow job for this annotation

GitHub Actions / Analyze (8.0.x)

Parameter 'Repository repository' is captured into the state of the enclosing type and its value is also used to initialize a field, property, or event.

Check warning on line 32 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View workflow job for this annotation

GitHub Actions / build (8.0.x)

Parameter 'Repository repository' is captured into the state of the enclosing type and its value is also used to initialize a field, property, or event.

/// <summary>
/// Fetches the content identified by the descriptor.
Expand Down Expand Up @@ -306,7 +306,7 @@
}

// 4. delete the dangling original referrers index, if applicable
await DeleteAsync(oldDesc, cancellationToken).ConfigureAwait(false);

Check warning on line 309 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View workflow job for this annotation

GitHub Actions / Analyze (8.0.x)

Possible null reference argument for parameter 'target' in 'Task ManifestStore.DeleteAsync(Descriptor target, CancellationToken cancellationToken = default(CancellationToken))'.

Check warning on line 309 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View workflow job for this annotation

GitHub Actions / build (8.0.x)

Possible null reference argument for parameter 'target' in 'Task ManifestStore.DeleteAsync(Descriptor target, CancellationToken cancellationToken = default(CancellationToken))'.
}

/// <summary>
Expand Down Expand Up @@ -397,5 +397,77 @@
/// <param name="cancellationToken"></param>
/// <returns></returns>
public async Task DeleteAsync(Descriptor target, CancellationToken cancellationToken = default)
=> await Repository.DeleteAsync(target, true, cancellationToken).ConfigureAwait(false);
=> await DeleteWithIndexing(target, cancellationToken).ConfigureAwait(false);

/// <summary>
/// DeleteWithIndexing deletes the specified target (Descriptor) from the repository,
/// handling referrer indexing if necessary.
/// </summary>
/// <param name="target">The target descriptor to delete.</param>
/// <param name="cancellationToken">A cancellation token to cancel the operation if needed. Defaults to default.</param>
/// <returns></returns>
internal async Task DeleteWithIndexing(Descriptor target, CancellationToken cancellationToken = default)
pat-pan marked this conversation as resolved.
Show resolved Hide resolved
{
switch (target.MediaType)
{
case MediaType.ImageManifest:
case MediaType.ImageIndex:
if (Repository.ReferrersState == Referrers.ReferrersState.Supported)
{
// referrers API is available, no client-side indexing needed
await Repository.DeleteAsync(target, true, cancellationToken).ConfigureAwait(false);
return;
}
var manifest = await FetchAsync(target, cancellationToken).ConfigureAwait(false);
pat-pan marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The manifest is not verified against the target descriptor. It is possible that a corrupted manifest is fetched.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to clarify that if this is to, for example, check the digests between target and manifest?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The download size of the manifest is not limited / well-guarded. It means it is vulnerable to excessive resource attack.

await IndexReferrersForDelete(target, manifest, cancellationToken).ConfigureAwait(false);
break;
}
await Repository.DeleteAsync(target, true, cancellationToken).ConfigureAwait(false);
}

/// <summary>
/// IndexReferrersForDelete indexes referrers for manifests with a subject field on manifest delete.
/// References:
/// - Latest spec: https://github.com/opencontainers/distribution-spec/blob/v1.1.0/spec.md#deleting-manifests
/// - Compatible spec: https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#deleting-manifests
pat-pan marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
/// <param name="target"></param>
/// <param name="manifestContent"></param>
/// <param name="cancellationToken"></param>
private async Task IndexReferrersForDelete(Descriptor target, Stream manifestContent, CancellationToken cancellationToken = default)
{
Descriptor subject;
switch (target.MediaType)
{
case MediaType.ImageManifest:
var imageManifest = JsonSerializer.Deserialize<Manifest>(manifestContent);
if (imageManifest?.Subject == null)
{
// no subject, no indexing needed
return;
}
subject = imageManifest.Subject;
break;
case MediaType.ImageIndex:
var imageIndex = JsonSerializer.Deserialize<Index>(manifestContent);
if (imageIndex?.Subject == null)
{
// no subject, no indexing needed
return;
}
subject = imageIndex.Subject;
break;
default:
return;

Check warning on line 461 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs#L461

Added line #L461 was not covered by tests
}

var isReferrersSupported = Repository.PingReferrers(cancellationToken);
if (isReferrersSupported)
{

Check warning on line 466 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs#L466

Added line #L466 was not covered by tests
// referrers API is available, no client-side indexing needed
return;

Check warning on line 468 in src/OrasProject.Oras/Registry/Remote/ManifestStore.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/ManifestStore.cs#L468

Added line #L468 was not covered by tests
}
await UpdateReferrersIndex(subject, new Referrers.ReferrerChange(target, Referrers.ReferrerOperation.Delete), cancellationToken)
.ConfigureAwait(false);
}
}
4 changes: 2 additions & 2 deletions src/OrasProject.Oras/Registry/Remote/Referrers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@
// limitations under the License.

using System.Collections.Generic;
using System.Linq;
using OrasProject.Oras.Content;
using OrasProject.Oras.Exceptions;
using OrasProject.Oras.Oci;

namespace OrasProject.Oras.Registry.Remote;
Expand All @@ -29,6 +27,8 @@ internal enum ReferrersState
}

internal record ReferrerChange(Descriptor Referrer, ReferrerOperation ReferrerOperation);

internal static readonly string ZeroDigest = "sha256:0000000000000000000000000000000000000000000000000000000000000000";
pat-pan marked this conversation as resolved.
Show resolved Hide resolved

internal enum ReferrerOperation
{
Expand Down
67 changes: 67 additions & 0 deletions src/OrasProject.Oras/Registry/Remote/Repository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@

private RepositoryOptions _opts;

private static readonly Object _referrersPingLock = new();

/// <summary>
/// Creates a client to the remote repository identified by a reference
/// Example: localhost:5000/hello-world
Expand Down Expand Up @@ -369,6 +371,71 @@
public async Task MountAsync(Descriptor descriptor, string fromRepository, Func<CancellationToken, Task<Stream>>? getContent = null, CancellationToken cancellationToken = default)
=> await ((IMounter)Blobs).MountAsync(descriptor, fromRepository, getContent, cancellationToken).ConfigureAwait(false);

/// <summary>
/// PingReferrers returns true if the Referrers API is available for the repository,
/// otherwise returns false
/// </summary>
/// <param name="cancellationToken"></param>
/// <returns></returns>
/// <exception cref="ResponseException"></exception>
/// <exception cref="Exception"></exception>
internal bool PingReferrers(CancellationToken cancellationToken = default)
pat-pan marked this conversation as resolved.
Show resolved Hide resolved
{
if (ReferrersState == Referrers.ReferrersState.Supported)
{
return true;

Check warning on line 386 in src/OrasProject.Oras/Registry/Remote/Repository.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/Repository.cs#L385-L386

Added lines #L385 - L386 were not covered by tests
}

if (ReferrersState == Referrers.ReferrersState.NotSupported)
{
return false;
}
pat-pan marked this conversation as resolved.
Show resolved Hide resolved

lock (_referrersPingLock)
{
// referrers state is unknown
// lock to limit the rate of pinging referrers API
if (ReferrersState == Referrers.ReferrersState.Supported)
{
return true;

Check warning on line 400 in src/OrasProject.Oras/Registry/Remote/Repository.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/Repository.cs#L399-L400

Added lines #L399 - L400 were not covered by tests
}

if (ReferrersState == Referrers.ReferrersState.NotSupported)
{
return false;

Check warning on line 405 in src/OrasProject.Oras/Registry/Remote/Repository.cs

View check run for this annotation

Codecov / codecov/patch

src/OrasProject.Oras/Registry/Remote/Repository.cs#L404-L405

Added lines #L404 - L405 were not covered by tests
}

var reference = Options.Reference.Clone();
reference.ContentReference = Referrers.ZeroDigest;
var url = new UriFactory(reference, Options.PlainHttp).BuildReferrersUrl();
var request = new HttpRequestMessage(HttpMethod.Get, url);
var response = Options.HttpClient.SendAsync(request, cancellationToken).ConfigureAwait(true).GetAwaiter()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we await this function and make PingReferrers async? But async calls are not allowed within the lock statement. We can try SemaphoreSlim.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The SemaphoreSlim does not seem right either. We need to consider a lock-free version.

.GetResult();

switch (response.StatusCode)
{
case HttpStatusCode.OK:
var supported = response.Content.Headers.ContentType?.MediaType == MediaType.ImageIndex;
SetReferrersState(supported);
return supported;
case HttpStatusCode.NotFound:
var err = (ResponseException)response.ParseErrorResponseAsync(cancellationToken)
.ConfigureAwait(true).GetAwaiter().GetResult();
if (err.Errors?.First().Code == ResponseException.ErrorCodeNameUnknown)
{
// referrer state is unknown because the repository is not found
throw err;
}

SetReferrersState(false);
return false;
default:
throw response.ParseErrorResponseAsync(cancellationToken).ConfigureAwait(true).GetAwaiter()
.GetResult();
}
}
}

/// <summary>
/// SetReferrersState indicates the Referrers API state of the remote repository. true: supported; false: not supported.
/// SetReferrersState is valid only when it is called for the first time.
Expand Down
21 changes: 21 additions & 0 deletions src/OrasProject.Oras/Registry/Remote/UriFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

using OrasProject.Oras.Exceptions;
using System;
using System.Web;

namespace OrasProject.Oras.Registry.Remote;

Expand Down Expand Up @@ -102,6 +103,26 @@ public Uri BuildRepositoryBlobUpload()
return builder.Uri;
}

/// <summary>
/// Builds the URL for accessing the Referrers API
/// Format: <scheme>://<registry>/v2/<repository>/referrers/<reference>?artifactType=<artifactType>
/// </summary>
/// <param name="artifactType"></param>
/// <returns></returns>
public Uri BuildReferrersUrl(string? artifactType = null)
{
var builder = NewRepositoryBaseBuilder();
builder.Path += $"/referrers/{_reference.ContentReference}";
if (!string.IsNullOrEmpty(artifactType))
{
var query = HttpUtility.ParseQueryString(builder.Query);
query.Add("artifactType", artifactType);
builder.Query = query.ToString();
}

return builder.Uri;
}

/// <summary>
/// Generates a UriBuilder with the base endpoint of the remote repository.
/// Format: <scheme>://<registry>/v2/<repository>
Expand Down
Loading
Loading