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

oras attach failing for registries that don't support 1.1-RC image media type #1224

Closed
1 task done
jlbutler opened this issue Jan 3, 2024 · 20 comments
Closed
1 task done
Labels
AWS ECR AWS ECR related issues bug Something isn't working enhancement New feature or request stale Inactive issues or pull requests

Comments

@jlbutler
Copy link

jlbutler commented Jan 3, 2024

What happened in your environment?

Opening a new issue to track oras attach failing to work for 1.0 registries (historically, issue #690 : oras attach on AWS ECR: Invalid JSON syntax). This was at one point working, so new issue to discuss more recent changes.

The changes to address issue #1043 in PR #1054 is where the client dropped the --image-spec option. There have been subsequent changes and some refactoring here and in oras-go since then.

What did you expect to happen?

With the --image-spec option removed, I would have expected the media type to be image.manifest.v1. However, without the option, attach uses image.manifest.v1.1 without any way to select 1.0.

How can we reproduce it?

With ECR:
oras attach --artifact-type some/example <registry-id>.dkr.ecr.<region>.amazonaws.com/<repo>:<tag> example.txt

Presumably, this is reproducible with any 1.0 registry that does not accept a 1.1 RC media type.

What is the version of your ORAS CLI?

1.1.0

What is your OS environment?

n/a

Are you willing to submit PRs to fix it?

  • Yes, I am willing to fix it.
@jlbutler jlbutler added bug Something isn't working triage New issues or PRs to be acknowledged by maintainers labels Jan 3, 2024
@shizhMSFT shizhMSFT added enhancement New feature or request AWS ECR AWS ECR related issues and removed triage New issues or PRs to be acknowledged by maintainers labels Jan 3, 2024
@shizhMSFT
Copy link
Contributor

shizhMSFT commented Jan 3, 2024

ECR strictly implements image-spec v1.1.0-rc2 with the subject field. ECR does not support the artifactType field, which is introduced since image-spec v1.1.0-rc3 and causing the "Invalid JSON syntax" problem. oras v1.1.0 implements image-spec v1.1.0-rc4 and oras attach dropped the support for previous RC versions of image-spec.

Note that oras v1.0.x works with ECR as it implements image-spec v1.1.0-rc2.

In summary, this issue is a feature request to bring back the --image-spec option and introduce options like v1.1, v1.1.0-rc2.

@FeynmanZhou
Copy link
Member

FeynmanZhou commented Jan 3, 2024

We chose to remove the flag --image-spec from oras attach in ORAS v1.1.0 because there was a breaking change in the OCI Image Spec v1.1.0-rc4 that removed the OCI Artifact Manifest. This has been discussed in #1043 and documented in the ORAS Spec.

For registries that implement image-spec v1.1.0-rc2, an workaround is to use ORAS v1.0.1 that allows to specify --image-spec v1.1-image with oras attach.

This sounds more like a registry compatibility issue with ORAS. To address this issue, I am fine with bringing --image-spec back to oras attach to increase the ORAS client compatibility.

@ktarplee
Copy link

ktarplee commented Jan 3, 2024

The slack conversation is here for reference.

@ktarplee
Copy link

ktarplee commented Jan 3, 2024

oras attach also stopped working for the Gitlab container registry. I assumed it was a gitlab regression but it is not been resolved in a few updates of Gitlab so it might be oras or an incompatibility between the two. This might be related to what you are seeing with ECR.

Error: PUT "https://reg.git.example.com/v2/ktarplee/test/telemetry/base/manifests/sha256:2ed182a012cd258381a134d4475d66460388fbeeb91333a3627d47a92ae6fc38": response status code 400: manifest blob unknown: blob unknown to registry

@vsoch
Copy link
Contributor

vsoch commented Jan 4, 2024

I just hit this issue for a push, and downgrading worked for me! Specifically:

wget https://github.com/oras-project/oras/releases/download/v1.0.1/oras_1.0.1_linux_amd64.tar.gz

# places ./oras in pwd
tar -xzvf oras_1.0.1_linux_amd64.tar.gz 

./oras push <container> <targz>
....
Pushed [registry] ......
Digest: sha256:a93a09035767b94185b20cdec3f7bdc5331425ff9a4ddf76e5c93febf3bea390

So it's fairly easy to grab an old release, extract to just the PWD, use as needed, and you are good :) But also we should hopefully have support for a fix in the newer versions (I have 1.0.1 installed on my system, which produced the error above).

@qweeah
Copy link
Contributor

qweeah commented Jan 4, 2024

I have 1.0.1 installed on my system, which produced the error above

@vsoch Do you mean 'which produced the logs above'?

@vsoch
Copy link
Contributor

vsoch commented Jan 4, 2024

@vsoch Do you mean 'which produced the logs above'?

No, I was referencing the error in the original issue.

jlbutler added a commit to jlbutler/oras that referenced this issue Jan 4, 2024
…atibility (oras-project#1224)

Signed-off-by: Jesse Butler <butlerjl@amazon.com>
jlbutler added a commit to jlbutler/oras that referenced this issue Jan 4, 2024
…atibility (oras-project#1224)

Signed-off-by: Jesse Butler <butlerjl@amazon.com>
@FeynmanZhou
Copy link
Member

FeynmanZhou commented Jan 4, 2024

oras attach also stopped working for the Gitlab container registry. I assumed it was a gitlab regression but it is not been resolved in a few updates of Gitlab so it might be oras or an incompatibility between the two. This might be related to what you are seeing with ECR.

Error: PUT "https://reg.git.example.com/v2/ktarplee/test/telemetry/base/manifests/sha256:2ed182a012cd258381a134d4475d66460388fbeeb91333a3627d47a92ae6fc38": response status code 400: manifest blob unknown: blob unknown to registry

@ktarplee From the error logs, it doesn't reveal the root cause so it is hard to determine that this is an incompatibility problem similar to this issue.

Could you try to paste your complete reproduced steps and debug log (with --debug)?

@vsoch
Copy link
Contributor

vsoch commented Jan 4, 2024

Here is the full error I was getting (sorry didn't realize it wasn't here, maybe I chose the wrong issue)!:

Error: PUT "xxxxxxxxxxxxxxx": response status code 405: unsupported: Invalid parameter at 'ImageManifest' failed to satisfy constraint: 'Invalid JSON syntax'

I think the underlying issue is the registry ECR is expecting the old manifest without the artifact.

@ktarplee
Copy link

ktarplee commented Jan 5, 2024

oras attach also stopped working for the Gitlab container registry. I assumed it was a gitlab regression but it is not been resolved in a few updates of Gitlab so it might be oras or an incompatibility between the two. This might be related to what you are seeing with ECR.

Error: PUT "https://reg.git.example.com/v2/ktarplee/test/telemetry/base/manifests/sha256:2ed182a012cd258381a134d4475d66460388fbeeb91333a3627d47a92ae6fc38": response status code 400: manifest blob unknown: blob unknown to registry

@ktarplee From the error logs, it doesn't reveal the root cause so it is hard to determine that this is an incompatibility problem similar to this issue.

Could you try to paste your complete reproduced steps and debug log (with --debug)?

@FeynmanZhou I looked into this a little more and I do think this is a unrelated issue. For instance, I just checked that if I use oras 1.0.1 the problem persists. So this is really a separate issue however at this point I do not know if the issue is with oras or with the gitlab registry implemention. When I get a few more hints at the problem I will open a new issue (unless you want me to open one now).

@sudo-bmitch
Copy link

sudo-bmitch commented Jan 5, 2024

So this is really a separate issue however at this point I do not know if the issue is with oras or with the gitlab registry implemention. When I get a few more hints at the problem I will open a new issue (unless you want me to open one now).

Doesn't GitLab have an allow list for the config mediaType? That would break the ability to push any artifact that isn't pre-approved by them. If they still have that policy, I'd just mark the registry as unsupported and focus on other issues.

Update: unless something has changed in the last few months, they still have an allow list for media types:
https://gitlab.com/gitlab-org/container-registry/-/merge_requests/1273

@jlbutler
Copy link
Author

After some offline discussions, we've aligned that chasing different RC versions of the 1.1 spec that happen to work with 1.0 registries is not the right approach. Eventually, the spec will be GA and we'll all move on with 1.0 or 1.1.

The remaining value of this issue is to consider if oras attach should work with 1.0 registries that disallow unknown fields in a manifest. I cannot speak for all registries, but I assume that some will remain 1.0, and some will continue to refuse unknown fields.

Note, I believe this is compliant 1.0 behavior. Image spec extensibility considerations state that registries "SHOULD NOT generate an error if they encounter an unknown property in a known media type". I don't think there's specification passage that indicates a registry MUST accept unknown fields (happy to be corrected).

So the decision to remove the --image-spec option boils down to a decision to remove support for these registries. If that is a position that the project has, then I think we can close this issue as will-not-fix.

@shizhMSFT @FeynmanZhou and other maintainers, feel free to make the call here and comment further, or close. Thanks!

@sudo-bmitch
Copy link

Note, I believe this is compliant 1.0 behavior. Image spec extensibility considerations state that registries "SHOULD NOT generate an error if they encounter an unknown property in a known media type". I don't think there's specification passage that indicates a registry MUST accept unknown fields (happy to be corrected).

I would treat rejecting an unknown property as generating an error. To me the spirit of the statement from OCI is to allow forward compatibility, new fields can be added without fear of every new field being a breaking change that requires an upgrade of builders, registries, and runtimes. So I don't believe this is OCI compliant.

I also feel that there are a number of non-OCI compliant registries out there, some intentionally so for good reasons. So it's useful for tooling to follow the robustness principle: "be conservative in what you send, be liberal in what you accept".

@jlbutler
Copy link
Author

I would treat rejecting an unknown property as generating an error.

Totally agree. I was more pointing at SHOULD NOT vs MUST NOT.

I also completely agree that the spirit of this passage is something implementors should follow, but I don't think you can say that a registry is non-compliant if it refuses unknown fields on a known media type.

I also feel that there are a number of non-OCI compliant registries out there, some intentionally so for good reasons

I think this is the main observation for maintainers here to consider. Not supporting this category of registries limits the use of oras attach, likely going forward beyond 1.1 GA.

@ktarplee
Copy link

ktarplee commented Mar 7, 2024

It looks like the latest version of Gitlab fixed their regression with oras attach. I issue we where seeing before is not longer an issue. It works again so I will assume it was a server side thing.

@bdehamer
Copy link

bdehamer commented Mar 8, 2024

It seems that there have been some recent changes on the AWS ECR side which almost make it compatible with oras attach.

When the artifact manifest is uploaded I see the following response:

Error: PUT "https://XXXXXXXXXXXX.dkr.ecr.us-east-1.amazonaws.com/v2...": response status code 405: unsupported: Invalid parameter at 'ImageManifest' failed to satisfy constraint: 'Invalid JSON syntax'

The manifest being uploaded looks something like:

{
  "annotations": {
    "org.opencontainers.image.created": "2024-03-08T19:12:54Z"
  },
  "artifactType": "application/vnd.dev.sigstore.bundle+json",
  "config": {
    "data": "e30=",
    "digest": "sha256:44136fa355b3678a1146ad16f7e8649e94fb4fc21fe77e8310c060f61caaff8a",
    "mediaType": "application/vnd.oci.empty.v1+json",
    "size": 2
  },
  "layers": [
    {
      "annotations": {
        "org.opencontainers.image.title": "att.json"
      },
      "digest": "sha256:d1c3842b99b57095dccfb71e590003e2a66c701a8eff20ffd7cba1ae07e3fd3d",
      "mediaType": "application/vnd.oci.image.layer.v1.tar",
      "size": 5752
    }
  ],
  "mediaType": "application/vnd.oci.image.manifest.v1+json",
  "schemaVersion": 2,
  "subject": {
    "digest": "sha256:221189b511bd1b01a233c9af11467cddb722347a2bef2162385b34034f78d212",
    "mediaType": "application/vnd.oci.image.index.v1+json",
    "size": 855
  }
}

However, I noticed that if I simply remove the config.data field from the request body, ECR will accept this manifest!

It seems like this style of embedded content is optional and up to the discretion of the client.

Is it worthwhile to add a flag to disable embedded content in order to achieve compatibility with AWS ECR?

Copy link

github-actions bot commented May 8, 2024

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale Inactive issues or pull requests label May 8, 2024
@ktarplee
Copy link

BTW, oras attach now works again on Gitlab (after the latest updates).

@github-actions github-actions bot removed the stale Inactive issues or pull requests label Jul 26, 2024
Copy link

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale Inactive issues or pull requests label Sep 24, 2024
Copy link

This issue was closed because it has been stalled for 30 days with no activity.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AWS ECR AWS ECR related issues bug Something isn't working enhancement New feature or request stale Inactive issues or pull requests
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants