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.

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

feat(credentials): Implement declarative stored credentials #857

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Josh-Matsuoka
Copy link
Contributor

Addresses #728

Creates a new credentials volume that is scanned for json representations of stored credentials, persisting them to the database if they aren't a duplicate.

Opening as draft for the moment pending the corresponding helm/operator implementations.

@Josh-Matsuoka Josh-Matsuoka added feat New feature or request safe-to-test labels Mar 21, 2025
@Josh-Matsuoka Josh-Matsuoka self-assigned this Mar 21, 2025
@github-actions github-actions bot added the needs-triage Needs thorough attention from code reviewers label Mar 21, 2025
@andrewazores andrewazores removed the needs-triage Needs thorough attention from code reviewers label Mar 21, 2025
private void createFromFile(java.nio.file.Path path) {
try (var is = new BufferedInputStream(Files.newInputStream(path))){
var credential = mapper.readValue(is, Credential.class);
var exists = Credential.find("id", credential.id).count() != 0;
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about checking for the IDs like this. The database will assign the IDs when the credentials are created, so I think this scheme can likely lead to duplicates getting created. If I manually create a Credential via API and it gets ID 1, then I shut the system down and put in a different declarative Credential definition with ID 1 into the directory, this will incorrectly reject the new declarative Credential even though it is actually new and different.

I think the right thing to do here is parse the declaratively defined Credentials, then check the database if any Credential exists with the exact same <username, password, matchExpression> - this tuple is what really defines the Credential.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants