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

Mark feature "support OCI Referrer API" as experimental feature #659

Closed
yizha1 opened this issue May 9, 2023 · 25 comments · Fixed by #663
Closed

Mark feature "support OCI Referrer API" as experimental feature #659

yizha1 opened this issue May 9, 2023 · 25 comments · Fixed by #663
Assignees
Labels
bug Something isn't working
Milestone

Comments

@yizha1
Copy link
Contributor

yizha1 commented May 9, 2023

What is the areas you experience the issue in?

Notation CLI

What is not working as expected?

Currently Referrer API is by default verified by Notation since v1.0.0-rc.4. For example, when using notation sign, if registries support referrer API, notation will not use an image index pushed to a tag described by the referrers tag schema. Since OCI spec 1.1 (including both image spec and distribution spec) is not officially released yet, support of Referrer API could lead to breaking changes or portability issue after Notation v1 is released. On the other hand, users may be confused that sometimes image index is pushed, sometimes not, which is an issue of consistency.

What did you expect to happen?

Mark feature "support OCI Referrer API" as experimental feature, and document it properly. Users can use experimental features only after env variable NOTATION_EXPERIMENTAL=1 is set.
The default behavior of Notation is always using an image index pushed to a tag described by the referrers tag schema. If users set env variable NOTATION_EXPERIMENTAL=1, Notation will check the support of OCI referrer API first, if registries support Referrer API, then notation will not push image index; if registries don't support Referrer API, notation will push image index as the default behavior.

How can we reproduce it?

N/A

Describe your environment

WSL

What is the version of your Notation CLI or Notation Library?

Notation v1.0.0-rc.4

@yizha1 yizha1 added bug Something isn't working triage Need to triage labels May 9, 2023
@yizha1
Copy link
Contributor Author

yizha1 commented May 9, 2023

@toddysm @iamsamirzon @sajayantony @shizhMSFT @vaninrao10 This issue was created per discussion during community call on 5/8/2023. Please take a look, and this issue should be addressed in Notation v1 release

@yizha1 yizha1 added this to the 1.0.0 milestone May 9, 2023
@yizha1 yizha1 removed the triage Need to triage label May 9, 2023
@Two-Hearts
Copy link
Contributor

Thanks @yizha1, I will take this one.

@Two-Hearts
Copy link
Contributor

I'm trying to summarize behaviors between Notation versions:
Sign as of Notation v1.0.0-rc.4 (current version):

  1. If store signature with image manifest: Try Referrers API first, if not supported, automatically fallback to referrers tag schema.
  2. If store signature with artifact manifest: ping Referrers API, if failed (i.e. Referrers API is not supported), then the whole Sign process is failed, no fallback would occur.

Verify as of Notation v1.0.0-rc.4 (current version):
Regardless of manifest type, always try Referrers API, if not supported, automatically fallback to referrers tag schema.

What we want in v1.0 for Sign:

  1. If experimental flag is disabled (NOTATION_EXPERIMENTAL=0), always use referrers tag schema to store signatures.
  2. If experimental flag is enabled (NOTATION_EXPERIMENTAL=1), then try referrers API first, if not supported, automatically fallback to referrers tag schema.

What we want in v1.0 for Verify:
Q: what is the expected behavior here? @yizha1 /cc: @iamsamirzon @vaninrao10 @sajayantony @toddysm @shizhMSFT @FeynmanZhou

@yizha1
Copy link
Contributor Author

yizha1 commented May 9, 2023

For notation verify in v1.0, the behavior is similar to notation sign. The default is using image index, if experimental flag is enabled (NOTATION_EXPERIMENTAL=1), then try referrers API first, if not supported, automatically fallback to use the image index. This applies to all the commands including notation sign, notation verify, notation ls, and notation inspect

@Two-Hearts
Copy link
Contributor

For notation verify in v1.0, the behavior is similar to notation sign. The default is using image index, if experimental flag is enabled (NOTATION_EXPERIMENTAL=1), then try referrers API first, if not supported, automatically fallback to use the image index. This applies to all the commands including notation sign, notation verify, notation ls, and notation inspect

(Just an open discussion here.) I'm not sure if this is what we expect, but in the below scenario:
Suppose the registry supports the Referrers API, user sets NOTATION_EXPERIMENTAL=1, then sign their artifact. The Sign succeeded without pushing the image index as Referrers API is used. Then, the user turns off experimental by setting NOTATION_EXPERIMENTAL=0. Next, if the user types notation list it would show empty, as Notation is default to referrers tag schema now. If the user types notation verify, the verification would fail with error: no signature is associated.... Now, if the user turns on the experimental again, both notation list and notation verify would work as Referrers API is turned on again.

In the above case, it seems that the result of list and verify depends on the value of NOTATION_EXPERIMENTAL. IMO, this might not be a very good user experience? So I think, maybe we could just put Referrers API behind experimental in Sign. For other commands, we still try Referrers API first, if failed, automatically fallback to tag schema.

@shizhMSFT
Copy link
Contributor

shizhMSFT commented May 9, 2023

We can assume the use of NOTATION_EXPERIMENTAL is fairly consistent and most customer will not enable this flag.

@iamsamirzon
Copy link
Contributor

2. rimental flag is enabled (NOTATION_EXPERIMENTAL=1), then try referrers API first, if not supported, automatically fallback to referrers tag schema.

I like the proposal from @Two-Hearts about having the Notation Verify check for referrers API first and if not available use the image-index method. As long as this does not cause inconsistent results for registries this may be a good path. Essentially be more flexible when verifying signatures so users do not have to change their deployment time checks (modify scripts or flags for verify) when their registries start supporting 1.1 based signatures

@priteshbandi
Copy link
Contributor

priteshbandi commented May 9, 2023

My 2 cents, for v1.0

For Sign:

  • If experimental flag is disabled (NOTATION_EXPERIMENTAL=0), always use referrers tag schema to store signatures. Also, customers should not be able to use --signature-manifest flag
  • If experimental flag is enabled (NOTATION_EXPERIMENTAL=1) and customer uses --signature-manifest artifact then try referrers API first, if not supported fail the signing. The default behavior without --signature-manifest flag should be same as with experimental disabled.

For Verify, inspect, list aka any command that only read's content from registry:

Always try Referrers API, if not supported, automatically fallback to referrers tag schema. --signature-manifest flag is not supported

Rationale:

  1. How signatures are stored should be a conscious call that user makes i.e. notation should not make any assumptions, which means for sign call we should take intent from users on what mechanism they prefer to store signature. This is required for cross tool compatibility.
  2. For operations such as verify, inspect, we can do automatic fallback. This provides backward compatibility for users who moves from signing using image-manifest to use artifact manifest.

IMO we should fix this ASAP as its breaking customer experience. Should we have RC5 with this fix?

@yizha1
Copy link
Contributor Author

yizha1 commented May 10, 2023

My 2 cents, for v1.0

For Sign:

  • If experimental flag is disabled (NOTATION_EXPERIMENTAL=0), always use referrers tag schema to store signatures. Also, customers should not be able to use --signature-manifest flag

The rationale for commands verify, ls and inspect makes sense. But Referrer API doesn't have to be with specific manifest type, that means, no matter what signature manifest is used. We have another issue to remove --signature-manifest flag, since artifact manifest has been removed by recent OCI image spec v1.0.0-rc.3 release.

@priteshbandi
Copy link
Contributor

priteshbandi commented May 10, 2023

@yizha1 Yes, we should remove --signature-manifest flag and introduce a new experimental flag --use-referrers-api (name TBD) using which users can control if they want to push a signature using tag-schema or referrers API.

Rationale:

  1. OCI spec removed artifact manifest.
  2. For backward compatibility in CI/CD tooling. Think about a scenario where a user is using a managed 3rd party registry like ACR, ECR, docker hub and has CICD tooling around it, a notation to sign an image, and a signature verification tool(either homegrown or 3rd party) that supports only tag schema to verify the image.
    As of today, the registry doesn't support referrers API so notation signs with tag schema and verification tool verify image using tag schema. One day, the registry added support for referrers API and suddenly notation started signing using referrers API, which the verification tool doesn't understand, and that broke the user's CICD.

@Two-Hearts
Copy link
Contributor

Two-Hearts commented May 10, 2023

Thanks @priteshbandi, to summarize what we want in RC.5:
For Sign:

  1. --signautre-manifest flag would be removed, we will not support signing with OCI artifact manifest anymore.
  2. Adding a new flag, --use-referrers-api (name TBD), behind experimental:
    • When both NOTATION_EXPERIMENTAL and --use-referrers-api are set, Notation would try the Referrers API first, if failed, automatically fallback to referrers tag schema.
    • Otherwise, Sign will be using referrers tag schema as default.

For other commands that consume signatures, notation list, notation verify, etc.:

  1. Notation still supports OCI artifact manifest.
  2. Always try the Referrers API first, if failed, automatically fallback to the referrers tag schema.

Does the above sound correct to you? (the new flag's name --use-referrers-api lgtm, but might need some further discussions.) /cc: @iamsamirzon @vaninrao10 @toddysm @sajayantony @shizhMSFT @yizha1 @FeynmanZhou

@shizhMSFT
Copy link
Contributor

shizhMSFT commented May 10, 2023

Always try the Referrers API first, if failed, automatically fallback to the referrers tag schema.

There might be a compatibility issue with some registries such as GAR.

Basically, attempting the Referrers API to GAR will not return a 404 but an unexpected 302.

See discussions:

@priteshbandi
Copy link
Contributor

priteshbandi commented May 10, 2023

@Two-Hearts

For Sign:
Adding a new flag, --use-referrers-api (name TBD), behind experimental:
When both NOTATION_EXPERIMENTAL and --use-referrers-api are set, Notation would try the Referrers API first, if failed, automatically fallback to referrers tag schema.
Otherwise, Sign will be using referrers tag schema as default

IMO we should not fallback for sign operation i.e. When both NOTATION_EXPERIMENTAL and --use-referrers-api are set, Notation would try the Referrers API only. If failed, fail the sign operation and ask user to not use --use-referrers-api flag.


@shizhMSFT
IMO automatic fallback would useful for most of the registries in OCI 1.0 to 1.1 migration usecase. For scenarios where registries are misbehaving not abiding by spec we can have --not-use-referrers-api(name TBD) flag.

@Two-Hearts
Copy link
Contributor

IMO we should not fallback for sign operation i.e. When both NOTATION_EXPERIMENTAL and --use-referrers-api are set, Notation would try the Referrers API only. If failed, fail the sign operation and ask user to not use --use-referrers-api flag.

Yeah, this also works. In this way, the Sign command becomes more deterministic (without any automatic fallback). Waiting for others' comments on this one.

@Two-Hearts
Copy link
Contributor

Two-Hearts commented May 10, 2023

Always try the Referrers API first, if failed, automatically fallback to the referrers tag schema.

There might be a compatibility issue with some registries such as GAR.

Basically, attempting the Referrers API to GAR will not return a 404 but an unexpected 302.

See discussions:

@shizhMSFT hmm, yes, we need to resolve issue 630. Do we want to have it done in RC.5 or v1.0?

@yizha1
Copy link
Contributor Author

yizha1 commented May 10, 2023

IMO we should not fallback for sign operation i.e. When both NOTATION_EXPERIMENTAL and --use-referrers-api are set, Notation would try the Referrers API only. If failed, fail the sign operation and ask user to not use --use-referrers-api flag.

Yeah, this also works. In this way, the Sign command becomes more deterministic (without any automatic fallback). Waiting for others' comments on this one.

Yes, it makes sense. Use the experimental flag means you want to leverage Referrer API, and not tag schema. Otherwise, it may succeed with tag schema, which is not what users expected.

@yizha1
Copy link
Contributor Author

yizha1 commented May 10, 2023

There might be a compatibility issue with some registries such as GAR.

Basically, attempting the Referrers API to GAR will not return a 404 but an unexpected 302.

See discussions:

Maybe we should raise it to OCI spec maintainers on the determination of referrer API

@shizhMSFT
Copy link
Contributor

In order to make notation stable while we are building notation on top of the instable distribution-spec, we need to identify which part of the distribution-spec is relatively stable and which is not. Apparently, the referrers tag schema is relatively stable and should be used as the default behavior of notation. Meanwhile, the referrers API and its fallback strategy is instable and should be marked as experimental.

Based on the above fundamentals, I'd like to propose a new experimental flag --allow-referrers-api to all registry related commands such as notation sign and notation verify.

  • When --allow-referrers-api is toggled with NOTATION_EXPERIMENTAL=1, notation attempts the referrers API and fallback to referrers tag schema on failure.
  • Otherwise, notation has deterministic behavior and uses referrers tag schema.

From previous discussions, we have consensus on using referrers tag schema by default for notation sign but not aligned on notation verify. To me, notation verify should not attempt referrers API without NOTATION_EXPERIMENTAL=1 since the referrers API itself is instable / experimental.

@Two-Hearts
Copy link
Contributor

In order to make notation stable while we are building notation on top of the instable distribution-spec, we need to identify which part of the distribution-spec is relatively stable and which is not. Apparently, the referrers tag schema is relatively stable and should be used as the default behavior of notation. Meanwhile, the referrers API and its fallback strategy is instable and should be marked as experimental.

Based on the above fundamentals, I'd like to propose a new experimental flag --allow-referrers-api to all registry related commands such as notation sign and notation verify.

  • When --allow-referrers-api is toggled with NOTATION_EXPERIMENTAL=1, notation attempts the referrers API and fallback to referrers tag schema on failure.
  • Otherwise, notation has deterministic behavior and uses referrers tag schema.

From previous discussions, we have consensus on using referrers tag schema by default for notation sign but not aligned on notation verify. To me, notation verify should not attempt referrers API without NOTATION_EXPERIMENTAL=1 since the referrers API itself is instable / experimental.

Yeah, after discussion with Shiwei offline, I agree with the above design. The automatic fallback from referrers API to tag schema is a part of the Referrers API's spec. As long as a user attempts the Referrers API, the process should include the fallback mechanism to align with the spec. From this point of view, I think we can still claim that Notation is deterministic regarding the use of Referrers API and Referrers tag schema.
@priteshbandi What do you think?

@FeynmanZhou
Copy link
Member

@shizhMSFT 's proposal makes sense to me. Using a new experimental flag --allow-referrers-api to all registry-related commands will provide a consistent experience for signing and verification.

My only concern is that this is a breaking change for users still using v1.0.0-RC.4 with Referrers API by default. They have to add this additional experimental flag --allow-referrers-api when verifying their legacy signatures after upgrading to v1.0.0-RC.4. We may need to document the considerations in the v1.0.0-RC.5 release note and elaborate on related docs.
@priteshbandi @Two-Hearts @yizha1 @sajayantony

@priteshbandi
Copy link
Contributor

priteshbandi commented May 12, 2023

@patrickzheng200 I agree, to summarize

If experimental flag is disabled (NOTATION_EXPERIMENTAL=0), dont use referrers API i.e.

  • For sign, only use referrers tag schema to store signatures.
  • For verify/inspect/list only use referrers tag schema to pull signatures.

If experimental flag is enabled (NOTATION_EXPERIMENTAL=1) and user uses --use-referrers flag; use oci 1.1 spec i.e. use referrers API

  • For sign, Check if referrers API returns 2xx or not. If it doesnt returns 2xx fail signing because registry hasnt correctly/fully implemented oci1.1. If it return 2xx then sign using referrers.
    • If referrers API returns 2xx, sign using referrers
    • If referrers API returns 404, fallback or the OCI-Subject header was not set, fallback and use tag schema
    • If referrers API returns anything else, fail the operation
  • For verify/inspect/list list, etc use referrers API first to pull the signature.
    • If referrers API returns 2xx use it to list signatures
    • If referrers API returns 404 or the OCI-Subject header was not set, fallback and use tag schema to list signatures
    • If referrers API returns anything else, fail the operation

@FeynmanZhou Had similar discussion with @toddysm on Thursday, IMO we should mark RC4 as deprecated ASAP and ask customers to directly jump to rc5 from rc3.

cc: @iamsamirzon

@Two-Hearts
Copy link
Contributor

If experimental flag is enabled (NOTATION_EXPERIMENTAL=1) and user uses --use-referrers flag; use oci 1.1 spec i.e. use referrers API

  • For sign, Check if referrers API returns 2xx or not. If it doesnt returns 2xx fail signing because registry hasnt correctly/fully implemented oci1.1. If it return 2xx then sign using referrers.

Thanks @priteshbandi, everything else LGTM. My only question is, when NOTATION_EXPERIMENTAL=1 and uses --allow-referrers flag during Sign, should we still enable fallback to referrers tag schema if referrers API is not supported? Because the fallback mechanism is a part of the Referrers API spec, trying referrers API without enabling the fallback schema seems not strictly following the distribution spec?
Quoting from the spec: If the [referrers API](https://github.com/opencontainers/distribution-spec/blob/main/spec.md#listing-referrers) returns a 404, the client MUST fallback to pulling the [referrers tag schema](https://github.com/opencontainers/distribution-spec/blob/main/spec.md#referrers-tag-schema).
Reference: https://github.com/opencontainers/distribution-spec/blob/main/spec.md#listing-referrers
/cc: @shizhMSFT

@priteshbandi
Copy link
Contributor

priteshbandi commented May 15, 2023

My interpretation (I am not closely following spec) of spec is that fallback is only for pull operation but since sign is an push operation its better to provide consistency . Also, if user is opting in for oci spec 1.1 we should always use referrers API related behaviors. Want to hear others opinion.

@yizha1
Copy link
Contributor Author

yizha1 commented May 15, 2023

My interpretation (I am not closely following spec) of spec is that fallback is only for pull operation but since sign is an push operation its better to provide consistency . Also, if user is opting in for oci spec 1.1 we should always use referrers API related behaviors. Want to hear others opinion.

@priteshbandi The pushing manifest procedure is defined here https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc2/spec.md#pushing-manifests-with-subject

@priteshbandi
Copy link
Contributor

priteshbandi commented May 15, 2023

Thanks @Two-Hearts and @yizha1 for pointing to the right spec. As oci1.1-rc2 calls out, when user adds NOTATION_EXPERIMENTAL=1 and uses --allow-referrers flag, notation should perform fallback for sign operation.


Updated #659 (comment)

priteshbandi pushed a commit that referenced this issue May 16, 2023
In this PR:
1. Removed Notation's support of signing with OCI artifact manifest. (One can still consume signatures in this type.)
2. Moved the Referrers API behind experimental as discussed in community meeting. This change takes effect on `sign`, `list`, `inspect`, and `verify` commands. When a user wants to try the Referrers API, they need to set both `NOTATION_EXPERIMENTAL=1` AND `--allow-referrers-api`; if the Referrers API is not supported, fallback to the Referrers tag schema automatically. Otherwise, by default, Notation would use the Referrers tag schema in all commands deterministically.
References: 
https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#listing-referrers
https://github.com/opencontainers/distribution-spec/blob/v1.1.0-rc1/spec.md#referrers-tag-schema
This PR has been tested and:
resolves #659
resolves #660

---------

Signed-off-by: patrick <[email protected]>
Signed-off-by: Patrick Zheng <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

7 participants