[RAPTOR-19721] feat(workload): update declared .env values and re-send rotated secrets - #1
Closed
wojtekwdr wants to merge 1 commit into
Closed
Conversation
…d rotated secrets --import-env adds names and never touches a value. This is the other half: a literal the manifest already declares is rewritten to match .env, and the credential behind a secret is re-sent, so a key rotated locally reaches the workload without a new credential, a new id or an edit to the manifest. A secret is re-sent without being compared first, because it cannot be: the platform never returns a stored value, so nothing can tell a rotated key from an untouched one, and a run reporting "no change" for a key you had just rotated would be worse than one request too many. Only a credential this CLI would have written is re-sent: the reference must name the apiToken field and the credential must carry the name setup gives it, because the store is tenant-wide and the platform accepts an apiToken write onto a credential of any type. The update route rejects credentialType, which the create route requires, so only the secret is sent. Verified against a live instance: with the field present every rotation fails with 422 "credentialType is not allowed key", which no mock would have shown. A re-sent secret reaches the workload when its containers next start, so the run says that outright and names the restart rather than leaving a rotation that looks applied. A re-send that fails is named too, and counted in the JSON envelope, because it leaves the old value serving, changes no file, and stops nothing. A plain run now also names a declared variable whose .env value no longer matches, the variables stored as credentials whose values cannot be compared at all, and the ones neither flag can settle: a value that is not a plain string, and a name whose kind has changed since it was written.
Owner
Author
|
Moved to datarobot-oss/cli. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
RATIONALE
Stacked on the
--import-envPR. That one adds names and never touches a value; this one is the opposite act, and it is the case the original report was about: a key rotated in.envhad no way to reach a workload short of creating a second credential by hand and editing the manifest to point at it.CHANGES
dr workload config --update-envrewrites a declared literal to match.env, and re-sends the credential behind a secret, keeping the id the manifest already names.apiTokenfield, and the credential must carry the name setup gives it.internal/workload.UpdateCredentialsends only the secret. The update route rejectscredentialType, which the create route requires.envSecretsRotated,envSecretsNotRotated), because it changes no file and stops nothing.TESTING
task testandtask lintpass. Exercised against a live instance, which is where thecredentialTyperejection turned up: with that field present every rotation fails with422 credentialType is not allowed key, and no mock shows it. After the fix, a rotation re-sends to the id already in the file, creates no second credential, leaves the manifest byte-identical, and the container serves the new value once it restarts. Also verified live: abasiccredential is refused rather than overwritten, and the platform does accept that write, so the guard is the only thing preventing it.NOTES
Second of a stack of three. Merge after the
--import-envPR.