Make default and external name interchangeable for install and update commands for ORAS repos
#989
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,13 +107,13 @@ Method ListModulesFromTagString( | |
| set allVersionsList = ..AggregatePlatformVersions($listfromstring(allTagsString, ", "), .aggregatedPlatformVersion) | ||
| set pointer = 0 | ||
| while $listnext(allVersionsList,pointer,moduleVersion) { | ||
| // filter by version | ||
| set tVersion = ##class(%IPM.General.SemanticVersion).FromString($$$Tag2Semver(moduleVersion)) | ||
| if 'tVersion.Satisfies(semverExpr) { | ||
| continue | ||
| } | ||
|
|
||
| // Special case: if we provided an explicit build number, require it. | ||
| if ##class(%IPM.General.SemanticVersion).IsValid(searchCriteria.VersionExpression,.specificVersion) && (specificVersion.Build '= "") && (specificVersion.Build '= tVersion.Build) { | ||
| continue | ||
| } | ||
|
|
@@ -126,11 +126,53 @@ Method ListModulesFromTagString( | |
| set tag = tag _ $$$OrasTagPlatformSeparator _ platformVersion | ||
| } | ||
|
|
||
| // get metadata from annotations | ||
| set metadata = ..GetPackageMetadata(..Location, name, tag, "", client) | ||
| set artifactMetadata = ##class(%IPM.Repo.Oras.ArtifactMetadata).%New() | ||
| $$$ThrowOnError(artifactMetadata.%JSONImport(metadata)) | ||
|
|
||
| // Filter by module name if requested - check both Name and ExternalName | ||
| if (searchCriteria.Name '= "") { | ||
| // Extract the package name without namespace prefix for comparison | ||
| // Package name from ORAS may be "namespace/modulename" or just "modulename" | ||
| set packageNameWithoutNamespace = $piece(name, "/", *) | ||
|
|
||
| // First check if package name already matches | ||
| set packageNameMatches = ($$$lcase(packageNameWithoutNamespace) = $$$lcase(searchCriteria.Name)) | ||
|
|
||
| // If packageNameMatches is true, proceed without additional checks | ||
| // Otherwise fetch module.xml to check namespaced Name and ExternalName | ||
| if 'packageNameMatches { | ||
| // Note: pass empty namespace since 'name' already includes the full path | ||
| set moduleXMLText = ..GetModuleXML(..Location, name _ ":" _ tag, "", ..Username, ..Password, ..Token, ..TokenAuthMethod) | ||
|
|
||
| if (moduleXMLText '= "") { | ||
| // Parse module.xml | ||
| try { | ||
| set moduleObj = ##class(%IPM.Utils.Module).GetModuleObjectFromString(moduleXMLText, .found) | ||
| } catch ex { | ||
| // if a single module.xml fails to parse, move onto the next one instead of failing completely | ||
| continue | ||
| } | ||
| if 'found { | ||
| continue | ||
| } | ||
|
|
||
| // Check if search name matches either Name or ExternalName (case-insensitive) | ||
| set matchesName = ($$$lcase(moduleObj.Name) = $$$lcase(searchCriteria.Name)) | ||
| set matchesExternalName = ((moduleObj.ExternalName '= "") && ($$$lcase(moduleObj.ExternalName) = $$$lcase(searchCriteria.Name))) | ||
|
|
||
| // Only include if at least one matches | ||
| if 'matchesName && 'matchesExternalName { | ||
| continue | ||
| } | ||
| } else { | ||
| // No module.xml available - skip this module | ||
| continue | ||
| } | ||
| } | ||
| } | ||
|
|
||
| set tModRef = ##class(%IPM.Storage.ModuleInfo).%New() | ||
| // `artifactMetadata.ImageTitle` can be different from `name`. E.g., when the module was simply "moved" from elsewhere under a different name. | ||
| set tModRef.Name = name | ||
|
|
@@ -162,48 +204,32 @@ Method ListModulesFromTagString( | |
| } | ||
| } | ||
|
|
||
| /// Lists the modules available in this repository that satisfy the search criteria | ||
| /// Note: for ORAS repositories only, the internal name and external name can be used interchangeably | ||
| Method ListModules(pSearchCriteria As %IPM.Repo.SearchCriteria) As %ListOfObjects(ELEMENTTYPE="%IPM.Storage.ModuleInfo") | ||
| { | ||
| // Get ORAS client | ||
| set client = ..GetClient(..Location, ..Username, ..Password, ..Token, ..TokenAuthMethod) | ||
|
|
||
| // Parse search criteria | ||
| set tVersionExpression = pSearchCriteria.VersionExpression | ||
| set tSC = ##class(%IPM.General.SemanticVersionExpression).FromString(pSearchCriteria.VersionExpression, .tVersionExpression) | ||
| $$$ThrowOnError(tSC) | ||
|
|
||
| // Collect list of modules | ||
| set tList = ##class(%Library.ListOfObjects).%New() | ||
|
|
||
| // Use /v2/_catalog to enumerate all packages, then filter appropriately | ||
|
Contributor
There was a problem hiding this comment. My memory is very rusty on this, but I think that not all ORAS registries support /v2/_catalog and the intent here was:
This would seem to break the first case. It would be good if you could dig on this a tiny bit to confirm.
Collaborator
Author
There was a problem hiding this comment. @isc-tleavitt Ok after a bit of digging, you're correct that /v2/_catalog is an optional spec that not every registry supports. Unfortunately the OCI spec does not have a replacement. Individual registries may implement their own version, for example Docker Hub has Not too sure what the correct approach is. I think we should use this endpoint if available, but then do we implement registry-specific handling if it is not?
Collaborator
Author
There was a problem hiding this comment. I've added some special handling to cover the case where /v2/_catalog is unavailable but name is specified. External name and default name won't be interchangeable in this case though
Contributor
There was a problem hiding this comment. @isc-dchui I don't want to do anything registry-specific, but if there is a generic way to work with a namespace and name, and without /v2/_catalog, I'd like to support it (as it seems we did prior to this change).
Contributor
There was a problem hiding this comment. Although I suppose as long as it works with zot and artifactory we're fine.
Collaborator
Author
There was a problem hiding this comment. Both zot and artifactory support the /v2/_catalog endpoint so we're good there at least |
||
| // Note: not every registry supports this endpoint, but without it, searching becomes much harder without using registry-specific endpoints | ||
| set request = ..GetHttpRequest() | ||
| // Make GET request | ||
| // response is a JSON structure like {"repositories":["package1", "package2", ...]} | ||
| set tSC=request.Get(..PathPrefix _ "/v2/_catalog") | ||
| $$$ThrowOnError(tSC) | ||
| set response=request.HttpResponse | ||
| if (response.StatusCode=200) { | ||
| // Handle results | ||
| set data = ##class(%DynamicAbstractObject).%FromJSON(response.Data) | ||
| set iter = data.repositories.%GetIterator() | ||
| // Iterate through each package | ||
| // key is the index | ||
|
|
@@ -213,22 +239,38 @@ Method ListModules(pSearchCriteria As %IPM.Repo.SearchCriteria) As %ListOfObject | |
| if (package="") { | ||
| continue | ||
| } | ||
|
|
||
| // If namespace is configured, only consider packages in that namespace | ||
| if (..Namespace '= "") { | ||
| set namespacePrefix = ..Namespace _ "/" | ||
| // Skip packages not in this namespace | ||
| if ($extract(package, 1, $length(namespacePrefix)) '= namespacePrefix) { | ||
| continue | ||
| } | ||
| } | ||
|
|
||
| // collect all versions for this package in tList | ||
| do ..ListModulesFromTagString(tVersionExpression, client, pSearchCriteria, package, .tList) | ||
| } | ||
| } else { | ||
| // Endpoint failed, but if the name is specified, still try to find it | ||
| set name = pSearchCriteria.Name | ||
| if (name '= "") { | ||
| if ..Namespace '= "" { | ||
| set name = ..AppendURIs(..Namespace, name) | ||
| } | ||
| do ..ListModulesFromTagString(tVersionExpression, client, pSearchCriteria, name, .tList) | ||
| } else { | ||
| // otherwise error out | ||
| set msg = "Error: Call to /v2/_catalog endpoint failed. This registry may not support it." | ||
isc-dchui marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| set msg = msg _ $char(10,13) _ "Response Code: "_response.StatusCode _ " - " _ response.ReasonPhrase | ||
| $$$ThrowStatus($$$ERROR($$$GeneralError, msg)) | ||
| } | ||
| } | ||
| return tList | ||
| } | ||
|
|
||
| /// ** ORAS FUNCTIONS ** | ||
| /// Returns an authenticated ORAS client | ||
| ClassMethod GetClient( | ||
| registry As %String, | ||
|
|
@@ -391,8 +433,8 @@ ClassMethod GetModuleXML( | |
| annotations = manifest.get('annotations', {}) | ||
| module_xml = annotations.get('com.intersystems.ipm.module.v1+xml') | ||
| if module_xml: | ||
| # remove extraneous whitespace | ||
| return module_xml.strip() | ||
| except Exception as ex: | ||
| print("Exception: %s" % str(ex)) | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort of a side question since it's not in your review directly, but why don't we check whether the version expression in the search criteria is valid first thing in this method and throw an error if not?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question! IsValid() checks whether it is a fully formed expression, e.g. 1.0.0, but the search criteria may include wildcards or other characters, e.g. 1.0.x., so we only want to make sure it is valid if it is a specific version