Npm/auth access #757
Npm/auth access #757
Conversation
moved login and token retrieval to credentials.npm
Mend Scan Summary: ❌Repository: open-component-model/ocm
|
|
Integration Tests for 2d1f934 run with result: Success ✅! |
|
Integration Tests for 2d1f934 run with result: Success ✅! |
|
Integration Tests for 0b2baf5 run with result: Success ✅! |
|
Integration Tests for 53f3617 run with result: Success ✅! |
|
|
||
| // BearerToken retrieves the bearer token for the given repository URL and package name. | ||
| // Either it's setup in the credentials or it will login to the registry and retrieve it. | ||
| func BearerToken(ctx cpi.ContextProvider, repoUrl string, pkgName string) (string, error) { |
There was a problem hiding this comment.
Those functions are typically in a technology support package (like pkg/helm). The identity packages are typically just used to describe the technology specific identity mapping for the uniform credential handling mechanism, so they deal with the identity and credential attributes and identity matchers used for the credential mechanism to find and describe credential sets. If you want to put technology specific access code into this identity package you should at least use different file names, not the standard names used for the mapping in the other packages to keep some kind of consistency. Better would be to put technology specific access code into a separate package to be used by access methods, blob access and uploader packages.
Basically all identity packages have some GetCredentials function taking the used identity parameters and providing found credential properties in a standard way. This method is then used in the technology support package to do technology specific things like you did in this method.
| @@ -96,11 +94,11 @@ func (a *AccessSpec) GetInexpensiveContentVersionIdentity(access accspeccpi.Comp | |||
| } | |||
|
|
|||
| func (a *AccessSpec) getPackageMeta(ctx accspeccpi.Context) (*meta, error) { | |||
| url := a.Registry + path.Join("/", a.Package, a.Version) | |||
There was a problem hiding this comment.
you moved this code in this method to change the reader method, but copied it into the reader method, also.
I think, this is, because you need parts of the attributes incorporated into the url as dedicated fields.
May be you should then add a method on the AccessSpec to calculate the URL to avoid the code duplication.
|
|
||
| // BearerToken retrieves the bearer token for the given repository URL and package name. | ||
| // Either it's setup in the credentials or it will login to the registry and retrieve it. | ||
| func BearerToken(ctx cpi.ContextProvider, repoUrl string, pkgName string) (string, error) { |
There was a problem hiding this comment.
I think to stay uniform with the existing packages, we should also move BearerToken() and Authorize() to the npm login file.
|
Integration Tests for 43431bb run with result: Success ✅! |
|
Integration Tests for 43431bb run with result: Success ✅! |
|
Integration Tests for 975a485 run with result: Success ✅! |
Description
When credentials are configured, let's also use them for the read access.
What type of PR is this? (check all applicable)
Related Tickets & Documents
Added tests?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Added to documentation?
Checklist: