Skip to content

Conversation

Skarlso
Copy link
Contributor

@Skarlso Skarlso commented Aug 26, 2025

What this PR does / why we need it

Fixes open-component-model/ocm-project#578

Which issue(s) this PR fixes

@github-actions github-actions bot added kind/feature new feature, enhancement, improvement, extension size/s Small size/m Medium labels Aug 26, 2025
@Skarlso Skarlso marked this pull request as ready for review August 26, 2025 07:28
@Skarlso Skarlso requested a review from a team as a code owner August 26, 2025 07:28
@Skarlso Skarlso marked this pull request as draft August 26, 2025 08:04
@Skarlso Skarlso force-pushed the support-oci-repositories branch from c1fbd25 to 817f238 Compare August 26, 2025 10:16
@Skarlso Skarlso force-pushed the support-oci-repositories branch from 4b1523f to e5b249f Compare August 28, 2025 07:44
addCMD.SetArgs([]string{
"add",
"component-version",
"--repository", fmt.Sprintf("http://%s", registryAddress),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jakobmoellerdev The "fix" to my above credential issue was adding http:// here before the repository address. 🤔 Is that correct or should it have worked without the scheme?

@Skarlso
Copy link
Contributor Author

Skarlso commented Aug 28, 2025

Ugh, the tests together are not working of course. I need a way to isolate them.

@Skarlso
Copy link
Contributor Author

Skarlso commented Aug 28, 2025

Yay, the tests are passing. I'm going to overhaul the suite_test a bit.

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
…rsing

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
@Skarlso Skarlso force-pushed the support-oci-repositories branch from 3db798e to 476c932 Compare August 29, 2025 06:03
@@ -106,7 +107,7 @@ add component-version --%[1]s ./path/to/%[2]s --%[3]s ./path/to/%[4]s.yaml
}

cmd.Flags().Int(FlagConcurrencyLimit, 4, "maximum number of component versions that can be constructed concurrently.")
file.VarP(cmd.Flags(), FlagRepositoryRef, string(FlagRepositoryRef[0]), LegacyDefaultArchiveName, "path to the repository")
cmd.Flags().StringP(FlagRepositoryRef, string(FlagRepositoryRef[0]), LegacyDefaultArchiveName, "repository specification")
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not actually a repository specification, is it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a repository url I guess?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also can be sort a specification because it's no longer "just a path". What could we call this?

func TestParseRepository(t *testing.T) {
tests := []struct {
name string
repoSpec string
Copy link
Contributor

Choose a reason for hiding this comment

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

It might be kind of a nit, but I think it's generally a bit misleading that you call this repoSpec, as a repo spec is a established term in ocm and that's not it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, we actually call this repoSpec in the Parser that gets it. 🤔 From the Ref we do:

	for _, prefix := range ValidPrefixes {
		token := "/" + prefix + "/"
		if idx := strings.LastIndex(input, token); idx != -1 {
			repoSpec := input[:idx]

On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
On-behalf-of: Gergely Brautigam <gergely.brautigam@sap.com>

Signed-off-by: Gergely Brautigam <182850+Skarlso@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature new feature, enhancement, improvement, extension size/l Large size/m Medium size/s Small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make ocm add cv --repository to support OCI registries
3 participants