Skip to content

feat: configurable preference for relative access creation for uploader #1412

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mandelsoft
Copy link
Contributor

@mandelsoft mandelsoft commented Apr 9, 2025

What this PR does / why we need it

By default the OCI uploader used to handle the implicit OCI uploads for OCI based OCM repositories
creates an absolute access method.

A new global attribute preferrelativeaccess (bool) can be used now to switch this behavior
to prefer a relative access method.

With the new config object local.oci.uploader.config.ocm.software it is possible to configure
host/port combinations for which the relative access should be preferred. (For example localhost).

type: local.oci.uploader.config.ocm.software
preferRelativeAccess: true
repositories:
- localhost
- other:5000

Which issue(s) this PR fixes

Fixes #1410

@mandelsoft mandelsoft requested a review from a team as a code owner April 9, 2025 11:10
@github-actions github-actions bot added area/documentation Documentation related kind/feature new feature, enhancement, improvement, extension size/l Large labels Apr 9, 2025
- use relative access methods for OCI upload to
  local OCM repository
- make preference configurable for repos
Copy link
Contributor

@jakobmoellerdev jakobmoellerdev left a comment

Choose a reason for hiding this comment

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

In general, relativeOciAccesses are not nicely specified and theyre already not part of the spec anymore. Nevertheless I recognize the value this gives for local debugging.

For me this PR is acceptable, but only if we mark the type as experimental somehow. I think relativeOciAccesses should be avoided because most tools will only know how to deal with ociArtifact / ociImage. Introducing yet another type that is only resolvable if a context is known is very dangerous.

// to a configuration struct of type T.
// A pointer to the configured struct is returned.
// Attention: T must be a struct type.
func GetConfigured[T any](ctxp ContextProvider) (*T, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

There is exactly one usage of this method, so I would like to keep it an internal helper

type RepositorySpec interface {
runtime.VersionedTypedObject
UniformRepositorySpecProvider
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is every Repository Spec now a Uniform Spec Provider?

)

const (
ATTR_KEY = "github.com/mandelsoft/ocm/preferrelativeaccess"
Copy link
Contributor

Choose a reason for hiding this comment

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

This attribute key should be changed.

Suggested change
ATTR_KEY = "github.com/mandelsoft/ocm/preferrelativeaccess"
ATTR_KEY = "ocm.software/ocm/oci/" + ATTR_SHORT

@@ -168,6 +168,27 @@ The following configuration types are supported:

At least the given values must be present in the certificate
to be accepted for a successful signature validation.
- <code>local.oci.uploader.config.ocm.software</code>
The config type <code>local.oci.uploader.config.ocm.software</code> can be used to set some
Copy link
Contributor

Choose a reason for hiding this comment

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

This should become part of the generic oci uploader configuration, e.g. oci.uploader.config.ocm.software because there is no semantic relationship between a "local.oci.uploader" and the relative access setup. (localhost is only used in the given examples and in tests but it could be any repository).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Documentation related kind/feature new feature, enhancement, improvement, extension size/l Large
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Using absolute access types for transferred-by-value artifacts is problematic
2 participants