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

Stop using *_STORAGE_API_ENDPOINT environment variables #759

Conversation

shreyas-s-rao
Copy link
Collaborator

What this PR does / why we need it:
As mentioned in #727, it is not secure to be passing credential (secret) information to the etcdbr process via environment variables. This is ok while running locally, but when running on a k8s cluster (deployed by etcd-druid), it becomes important for etcdbrctl to be able to fetch the information about endpoint overrides directly from the mounted secret file, where the other default credentials already exist today.

This PR does two things:

  1. For provider GCP, adds support for specifying storageAPIEndpoint via file path, while still supporting the environment variable GOOGLE_STORAGE_API_ENDPOINT env var (for users who may already be using it.
  2. For provider Azure, removes support for AZURE_STORAGE_API_ENDPOINT and now uses field domain, in-line with Add support to override Azure Blob Storage Domain #756 . This now allows users to use custom domains for accessing special Azure region services, like this.

Which issue(s) this PR fixes:
Fixes #727

Special notes for your reviewer:
/assign @renormalize @unmarshall
/cc @AleksandarSavchev

Release note:

Add support for specifying Google storage API endpoint via file `~/.gcp/storageAPIEndpoint`. Environment variable `GOOGLE_STORAGE_API_ENDPOINT` is deprecated, and will be removed shortly.
Add support for specifying custom domains for Azure storage. 
Remove support for specifying Azure custom endpoint via environment variable `AZURE_STORAGE_API_ENDPOINT`. Please use the new `domain` field (via JSON or file) instead.

@shreyas-s-rao shreyas-s-rao added area/security Security related area/usability Usability related kind/task General task labels Aug 12, 2024
@shreyas-s-rao shreyas-s-rao added this to the v0.30.0 milestone Aug 12, 2024
@shreyas-s-rao shreyas-s-rao requested a review from a team as a code owner August 12, 2024 00:00
@gardener-robot gardener-robot added the needs/review Needs review label Aug 12, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 12, 2024
@gardener-robot gardener-robot added the size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) label Aug 12, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 added needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 12, 2024
docs/deployment/getting_started.md Show resolved Hide resolved
docs/deployment/getting_started.md Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Show resolved Hide resolved
pkg/snapstore/abs_snapstore.go Show resolved Hide resolved
pkg/snapstore/gcs_snapstore.go Show resolved Hide resolved
pkg/snapstore/gcs_snapstore.go Outdated Show resolved Hide resolved
@gardener-robot gardener-robot added the needs/changes Needs (more) changes label Aug 12, 2024
@gardener-robot-ci-1 gardener-robot-ci-1 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 12, 2024
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 12, 2024
Copy link
Member

@renormalize renormalize left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @shreyas-s-rao. Other than the comments #759 (comment), #759 (comment), the PR looks to be in good shape.

pkg/snapstore/gcs_snapstore.go Outdated Show resolved Hide resolved
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Aug 12, 2024
@shreyas-s-rao shreyas-s-rao self-assigned this Aug 14, 2024
Copy link
Contributor

@unmarshall unmarshall left a comment

Choose a reason for hiding this comment

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

Since you wanted to limit the scope of this PR, hence i have resolved all my comments.
/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/changes Needs (more) changes needs/review Needs review labels Aug 14, 2024
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Aug 14, 2024
Copy link
Member

@renormalize renormalize left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, @shreyas-s-rao!

@shreyas-s-rao shreyas-s-rao merged commit 6428ff7 into gardener:master Aug 16, 2024
9 checks passed
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Aug 16, 2024
@shreyas-s-rao shreyas-s-rao deleted the task/remove-storage-endpoint-env-var branch August 16, 2024 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security Security related area/usability Usability related kind/task General task needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) size/m Size of pull request is medium (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Do not store Kubernetes secrets as environment variables
7 participants