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.

Sign up for GitHub

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

Jump to bottom

Bug: + in component version is replaced by .build- #1235

Open
hilmarf opened this issue Jan 9, 2025 · 9 comments · May be fixed by #1243
Open

Bug: + in component version is replaced by .build- #1235

hilmarf opened this issue Jan 9, 2025 · 9 comments · May be fixed by #1243
Assignees
@hilmarf
Labels
area/ipcei Important Project of Common European Interest kind/bugfix Bug
Milestone
2025-Q1

Comments

@hilmarf
Copy link
Member

hilmarf commented Jan 9, 2025 •
edited
Loading

What happened:

During ocm transfer the cv version: 0.0.1-20250108132333+af79499, gets changed to: 0.0.1-20250108132333.build-af79499

What you expected to happen:

I didn't expect that + is replaced by .build-. Why is that happening fully automatic?

How to reproduce it (as minimally and precisely as possible):

--- # component-constructor.yaml
components:
  - name: docker.io/recmo/docker-hello-tiny
    version: 0.0.1-20250108132333+af79499
    provider:
      name: recmo
    resources:
      - name: docker-hello-tiny
        version: v1.0
        type: ociImage
        access:
          type: ociArtifact
          imageReference: docker.io/recmo/docker-hello-tiny:latest
ocm add cv -c -F .\tmp\ .\component-constructor.yaml
ocm transfer cv .\tmp\ ocm-oci-test.int.repositories.cloud.sap

Error:

ocm get cv ocm-oci-test.int.repositories.cloud.sap//docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499 -o yaml

closing session: unable to close: closing docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499: unable to unref last: unable to cleanup component version docker.io/recmo/docker-hello-tiny/0.0.1-20250108132333.build-af79499 while unref last: closing component version docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499: check failed: component version "0.0.1-20250108132333+af79499" is invalid

Environment:
@hilmarf hilmarf added the kind/bugfix Bug label Jan 9, 2025
@github-actions github-actions bot added the area/ipcei Important Project of Common European Interest label Jan 9, 2025
@hilmarf hilmarf added this to the 2025-Q1 milestone Jan 9, 2025
@hilmarf hilmarf moved this from 🆕 ToDo to 🏗 In Progress in OCM Backlog Board Jan 9, 2025
@hilmarf hilmarf self-assigned this Jan 9, 2025
@ikhandamirov
Copy link
Contributor

I suppose, the '+' character is not allowed in the version as per specification:

Image tags consist of lowercase and uppercase letters, digits, underscores (_), periods (.), and dashes (-). It can be up to 128 characters long. And must follow the next regex pattern: [a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}

and therefore getting replaced. See also: https://kubernetes.io/docs/concepts/containers/images/

@ikhandamirov
Copy link
Contributor

Same in the OCI spec:

Throughout this document, <reference> as a tag MUST be at most 128 characters in length and MUST match the following regular expression:

[a-zA-Z0-9_][a-zA-Z0-9._-]{0,127}

https://github.com/opencontainers/distribution-spec/blob/main/spec.md#workflow-categories

@hilmarf
Copy link
Member Author

hilmarf commented Jan 10, 2025

@ikhandamirov thanks for those!

Weird, why OCM isn't following here semver... semver.org clearly allows + as separator for Build metadata.

@morri-son
Copy link
Contributor

@hilmarf I'm not able to reproduce the issue. I can create, transfer and then get the CV from the target registry with the version 0.0.1-20250108132333+af79499 you specified in your example:

❯ cat comp.yaml
components:
- name: ocm.software/test2
  version: 0.0.1-20250108132333+af79499
  provider:
    name: internal
  resources:
    - name: docker-hello-tiny
      version: v1.0
      type: ociImage
      access:
        type: ociArtifact
        imageReference: docker.io/recmo/docker-hello-tiny:latest

❯ ocm add cv -c --file ./ctf comp.yaml
processing comp.yaml...
  processing document 1...
    processing index 1
found 1 component
adding component ocm.software/test2:0.0.1-20250108132333+af79499...
  adding resource ociImage: "name"="docker-hello-tiny","version"="v1.0"...

❯ ocm transfer ctf ./ctf ocm-test.int.repositories.cloud.sap/d032990
transferring component "ocm.software/test2"...
  transferring version "ocm.software/test2:0.0.1-20250108132333+af79499"...
  ...adding component version...

❯ ocm get cv ocm-test.int.repositories.cloud.sap/d032990//ocm.software/test2:0.0.1-20250108132333+af79499 -oyaml
---
component:
  componentReferences: []
  creationTime: "2025-01-10T13:01:55Z"
  name: ocm.software/test2
  provider: internal
  repositoryContexts:
  - baseUrl: ocm-test.int.repositories.cloud.sap
    componentNameMapping: urlPath
    subPath: d032990
    type: OCIRegistry
  resources:
  - access:
      imageReference: docker.io/recmo/docker-hello-tiny:latest
      type: ociArtifact
    digest:
      hashAlgorithm: SHA-256
      normalisationAlgorithm: ociArtifactDigest/v1
      value: c6be339a02a039f6ceac6b749568a2b4e76c619a2827e11b46da8745e72cd7af
    name: docker-hello-tiny
    relation: external
    type: ociImage
    version: v1.0
  sources: []
  version: 0.0.1-20250108132333+af79499
meta:
  schemaVersion: v2

@hilmarf
Copy link
Member Author

hilmarf commented Jan 10, 2025

@morri-son, that's maybe the case because the version gets converted back, when calling ocm get. But please check your component-descriptors in the repo:

Image

@hilmarf
Copy link
Member Author

hilmarf commented Jan 10, 2025 

ocm get cv ocm-oci-test.int.repositories.cloud.sap//docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499 -o yaml

fails for me with:

Error: closing session: unable to close: closing docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499: unable to unref last: unable to cleanup component version  docker.io/recmo/docker-hello-tiny/0.0.1-20250108132333.build-af79499 while unref last: closing component version docker.io/recmo/docker-hello-tiny:0.0.1-20250108132333.build-af79499: check failed: component version "0.0.1-20250108132333+af79499" is invalid

although the returned yaml looks like this:

---
component:
  componentReferences: []
  creationTime: "2025-01-09T11:20:47Z"
  name: docker.io/recmo/docker-hello-tiny
  provider: recmo
  repositoryContexts:
  - baseUrl: ocm-oci-test.int.repositories.cloud.sap
    componentNameMapping: urlPath
    type: OCIRegistry
  resources:
  - access:
      imageReference: ocm-oci-test.int.repositories.cloud.sap/recmo/docker-hello-tiny:latest@sha256:c6be339a02a039f6ceac6b749568a2b4e76c619a2827e11b46da8745e72cd7af
      type: ociArtifact
    digest:
      hashAlgorithm: SHA-256
      normalisationAlgorithm: ociArtifactDigest/v1
      value: c6be339a02a039f6ceac6b749568a2b4e76c619a2827e11b46da8745e72cd7af
    name: docker-hello-tiny
    relation: external
    type: ociImage
    version: v1.0
  sources: []
  version: 0.0.1-20250108132333+af79499
meta:
  schemaVersion: v2

@morri-son
Copy link
Contributor

ok, but aren't we then look at OCI tags and not OCM component versions? Since the OCI spec does not allow +signs in tags and OCM components are OCI artifacts, the version needs to be converted into a valid tag. And as long as interacting with OCM component versions happens using the lib or the CLI, the end user always works with the same version used when creating the component. Maybe it is worth to add a log message when anything is converted, so that end users using a UI like shown in your comment can related the result to the required conversion.

@hilmarf
Copy link
Member Author

hilmarf commented Jan 10, 2025

PR which introduced this:
#355

hilmarf added a commit to hilmarf/ocm that referenced this issue Jan 10, 2025
@hilmarf
fixes: open-component-model#1235
2dc0134
@hilmarf hilmarf linked a pull request Jan 10, 2025 that will close this issue
Open
@mandelsoft
Copy link
Contributor

The problem is, that a version with this .build- is not a valid version anymore for CVs. So, the error is basically correct, but it comes far too late.

It must be assured that the version name given at the API matches the version name stored in the component descriptor, otherwise the complete structure would be corrupted.

So, such a version should be rejected at all.

May be you could add the following check to LookupVersion in api/ocm/cpi/repocpi/bridge_c.go Line 102

func (b *componentAccessBridge) LookupVersion(version string) (cpi.ComponentVersionAccess, error) {
	i, err := b.impl.LookupVersion(version)
	if err != nil {
		return nil, err
	}
	if i == nil || i.Impl == nil {
		return nil, errors.ErrInvalid("component implementation behaviour", "LookupVersion")
	}
	if i.Impl.GetDescriptor().Version != version {
		i.Impl.Close()
		return nil, errors.ErrInvalid("component version mismatch")
	}
	return NewComponentVersionAccess(b.GetName(), version, i.Impl, i.Lazy, i.Persistent, !compositionmodeattr.Get(b.GetContext()))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@hilmarf hilmarf

Labels
area/ipcei Important Project of Common European Interest kind/bugfix Bug
Projects
OCM Backlog Board
Status: 🏗 In Progress
Milestone
2025-Q1
4 participants
@mandelsoft @hilmarf @morri-son @ikhandamirov