Skip to content

Support for file store authentication (docker login)#56

Merged
jonesbusy merged 24 commits into
oras-project:mainfrom
AayushSaini101:15
Feb 17, 2025
Merged

Support for file store authentication (docker login)#56
jonesbusy merged 24 commits into
oras-project:mainfrom
AayushSaini101:15

Conversation

@AayushSaini101
Copy link
Copy Markdown
Contributor

Description

Added new support fetching credentials from file storage.
Explanation: Created a class File Store that helps to store the credentials in the particular config.

Closes #15

Testing done

Added test cases to verify the methods of the class

### Submitter checklist
- [x] If an issue exists, it is well described and linked in the description
- [x] The description of this pull request is detailed and explain why this pull request is needed
- [ ] The changeset is on a specific branch. Using `feature/` for new feature, or improvements ; Using `fix/` for bug fixes ; Using `docs/` for any documentation changes.
- [ ] If required, the documentation has been updated
- [ ] There is automated tests to cover the code change / addition or an explanation why there is no tests in the description.

@jonesbusy
Copy link
Copy Markdown
Collaborator

Thanks

How to use this to authenticate to OCI registry?

Would be nice to have some example (documentation) and ideally integration test (with test container).

I'm fine to merge partial implementation, but I think it doesn't solve #15 if we cannot use this new authentication method.

Comment thread src/main/java/land/oras/credentials/FileStore.java Outdated
@jonesbusy
Copy link
Copy Markdown
Collaborator

https://github.com/oras-project/oras-java/tree/main/src%2Fmain%2Fjava%2Fland%2Foras%2Fauth

I would expect to see a new implementation here. That get the credentials from the file and use it to perform request against the registry

@AayushSaini101
Copy link
Copy Markdown
Contributor Author

https://github.com/oras-project/oras-java/tree/main/src%2Fmain%2Fjava%2Fland%2Foras%2Fauth

I would expect to see a new implementation here. That get the credentials from the file and use it to perform request against the registry

Sure, i will add, also i have added some test case to verify the loadConfig https://github.com/oras-project/oras-java/pull/56/files#diff-06cef6e45e7cf319c09fb997438c922de0ce1f1b256a09826acb23267dcaaeefR113:~:text=testConfigLoad_success()%20throws-,Exception,-%7B

Comment thread pom.xml Outdated
Comment thread pom.xml Outdated
Comment thread pom.xml Outdated
Comment thread src/main/java/land/oras/credentials/FileStore.java Outdated
@jonesbusy
Copy link
Copy Markdown
Collaborator

Thanks. In my opinion adding Jackson dependency is not necessary. We use JSONUtils to serialize object from/to.

Internally it used GSON (this might change in the future).

What is the reason to add Jackson? Why is a compile scope and not test scope?

Thanks

@AayushSaini101
Copy link
Copy Markdown
Contributor Author

Thanks. In my opinion adding Jackson dependency is not necessary. We use JSONUtils to serialize object from/to.

Internally it used GSON (this might change in the future).

What is the reason to add Jackson? Why is a compile scope and not test scope?

Thanks

@jonesbusy Use JSONutils instead of jackson thanks

Comment thread src/main/java/land/oras/utils/JsonUtils.java Outdated
Comment thread pom.xml
Comment thread pom.xml
@jonesbusy
Copy link
Copy Markdown
Collaborator

Do you think we can have a test on https://github.com/oras-project/oras-java/blob/main/src/test/java/land/oras/RegistryTest.java

Right now they don't user any authentication, but I would be great to have a test with an existing auth file and ensuring it's read automatically if present.

Perhaps it should be the default when we don't provide an authentication to the SDK

@AayushSaini101
Copy link
Copy Markdown
Contributor Author

https://github.com/oras-project/oras-java/blob/main/src/test/java/land/oras/RegistryTest.java

yes, we can create another issue to create test cases for different authentication present in java-sdk, what do you think ? i guess creating another issue make sense

@AayushSaini101
Copy link
Copy Markdown
Contributor Author

@jonesbusy Added implementation of FileStore, the PR will be very large we can partially merge this what do you think ?

Comment thread src/main/java/land/oras/auth/FileStoreAuthenticationProvider.java Outdated
Comment thread src/main/java/land/oras/auth/FileStoreAuthenticationProvider.java
@jonesbusy
Copy link
Copy Markdown
Collaborator

There is no rush to merge this PR. I don't see it as a large PR to read and review

It looks better now with AuthProvider but still need some cleanup to remove code duplicate, getter that alter state etc...

Thanks for your contribution

@AayushSaini101
Copy link
Copy Markdown
Contributor Author

There is no rush to merge this PR. I don't see it as a large PR to read and review

It looks better now with AuthProvider but still need some cleanup to remove code duplicate, getter that alter state etc...

Thanks for your contribution

sure, i will add more test case in the registry as well thanks : )

Comment thread src/main/java/land/oras/credentials/FileStore.java
Comment thread src/main/java/land/oras/auth/FileStoreAuthenticationProvider.java Outdated
AayushSaini101 and others added 20 commits February 17, 2025 13:50
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Co-authored-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
…ject#86)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
Signed-off-by: Valentin Delaye <jonesbusy@users.noreply.github.com>
Signed-off-by: AayushSaini101 <kumaraayush9810@gmail.com>
@jonesbusy jonesbusy self-assigned this Feb 17, 2025
@jonesbusy
Copy link
Copy Markdown
Collaborator

Thanks for the effort. LGTM!

@jonesbusy jonesbusy changed the title feat: Add new method to fetch the credentials from the fileStore Support for file store authentication (docker login) Feb 17, 2025
@jonesbusy jonesbusy merged commit 2010509 into oras-project:main Feb 17, 2025
@jonesbusy jonesbusy added enhancement New feature or request and removed chore labels Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support reading credentials from file store

3 participants