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

[BUG] Temporarily enable passing of cloud provider credentials in JSON through the directory pointed by ${PROVIDER}_APPLICATION_CREDENTIALS #729

Closed
unmarshall opened this issue Apr 25, 2024 · 3 comments · Fixed by #730
Assignees
Labels
kind/bug Bug status/closed Issue is closed (either delivered or triaged)
Milestone

Comments

@unmarshall
Copy link
Contributor

unmarshall commented Apr 25, 2024

Describe the bug:

Example provides 2 options to specify the backup bucket secret. Option 2 (with data as JSON) is not supported today and should be removed.

NOTE: Please make the correction for all object store providers.

This only creates confusion for adopters. See https://kubernetes.slack.com/archives/CB57N0BFG/p1713778819927999

@unmarshall unmarshall added the kind/bug Bug label Apr 25, 2024
@ishan16696
Copy link
Member

This feature was introduced in this PR: #435, at that time backup-restore was also consumed by community via helm-charts(standalone) without etcd-druid.
In this PR: gardener/etcd-druid#301, I integrated this feature with etcd-druid and if you check the release note of this PR, it only mention about passing secrets via file path <ProviderName>_APPLICATION_CREDENTIALS , not via JSON format as it druid never supported this JSON format secret.
Reason:
How's etcd-druid get to know that storage secrets are in json format or not ?
StorageProviderName is a part of etcd spec but not the format of provider secret .That's why may be I decided not to make it more complicated and limit the json format only to backup-restore.
I hope you got the background of the issue.

Example provides 2 options to specify the backup bucket secret. Option 2 (with data as JSON) is not supported today and should be removed.

It should be not removed as community might be consuming it (those who are running backup-restore standalone). We can just harmonize the both type of secrets format in backup-restore.

@shreyas-s-rao shreyas-s-rao added this to the v0.29.0 milestone Apr 25, 2024
@shreyas-s-rao
Copy link
Collaborator

/assign @renormalize

@renormalize renormalize changed the title [BUG] Incorrect documentation for defining backup bucket secret [BUG] Temporarily enable passing of cloud provider credentials in JSON through the directory pointed by PROVIDER_APPLICATION_CREDENTIALS Apr 28, 2024
@renormalize renormalize changed the title [BUG] Temporarily enable passing of cloud provider credentials in JSON through the directory pointed by PROVIDER_APPLICATION_CREDENTIALS [BUG] Temporarily enable passing of cloud provider credentials in JSON through the directory pointed by ${PROVIDER}_APPLICATION_CREDENTIALS Apr 28, 2024
@renormalize
Copy link
Member

It was decided to temporarily enable passing of cloud provider credentials to etcd-backup-restore as a JSON file which resides in the directory pointed to by ${PROVIDER}_APPLICATION_CREDENTIALS, since members of the community were passing credentials through a JSON file, while using the directory method of passing credentials to etcd-backup-restore.

Though this specific method was never supported by etcd-backup-restore, enabling of passing credentials to etcd-backup-restore through two different formats, i.e. directory with individual files, and a singular JSON file, caused confusion in the community while using gardener/etcd-druid.

Only ${PROVIDER}_APPLICATION_CREDENTIALS is supported by etcd-druid, and examples present in etcd-backup-restore of passing credentials in JSON caused community members to pass credentials as a JSON file, through the directory while using etcd-druid, which caused etcd-backup-restore to error.

Therefore, after discussion, the maintainers have decided to deprecate passing credentials in a JSON format to etcd-backup-restore, and only support credentials through individual files in a directory. This will be deprecated 3 releases from now, i.e. v0.31.0 even though the project is only in alpha.

This will make the both projects aligned on the way credentials are passed.

Please include the intent to deprecate JSON credentials in the release notes for v0.29.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Bug status/closed Issue is closed (either delivered or triaged)
Projects
None yet
5 participants