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

install: Enable user-provided OIDC credentials secrets #949

Merged

Conversation

ironcladlou
Copy link
Contributor

@ironcladlou ironcladlou commented Feb 1, 2022

This commit makes it possible for install command users to specify a
pre-defined OIDC S3 credentials secret and key name using the new
--oidc-storage-provider-s3-secret and --oidc-storage-provider-s3-secret-key
flags. The existing --oidc-storage-provider-s3-credentials flag (which creates
the secret on the user's behalf) is unaffected.

This is important because users provisioning hypershift operator inputs (e.g.
credentials) most likely don't want the static manifests rendered by install
to include secret data. Most likely they are provisioning that resource
separately and simply want it referenced.

This commit also cleans up the operator deployment to stop reading the OIDC
bucket name and region from the OIDC secret. Those details are already
explicitly provided by the --oidc-storage-provider-s3-region and
--oidc-storage-provider-s3-bucket-name flags, and reading them from the secret
is both unnecessary and imposes a weird contract on end users bringing their own
secret (who would have to repeat the bucket name and region as "special" keys in
their credentials secret).

Finally, this commit takes the kube-system/oidc-storage-provider-s3-config
configmap out of the install output, as this resource is now published
automatically by the hypershift-operator.

Checklist

  • Subject and description added to both, commit and PR.
  • Relevant issues have been referenced.
  • This change includes docs.
  • This change includes unit tests.

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 1, 2022
This commit makes it possible for `install` command users to specify a
pre-defined OIDC S3 credentials secret and key name using the new
`--oidc-storage-provider-s3-secret` and `--oidc-storage-provider-s3-secret-key`
flags. The existing `--oidc-storage-provider-s3-credentials` flag (which creates
the secret on the user's behalf) is unaffected.

This is important because users provisioning hypershift operator inputs (e.g.
credentials) most likely don't want the static manifests rendered by `install`
to include secret data. Most likely they are provisioning that resource
separately and simply want it referenced.

This commit also cleans up the operator deployment to stop reading the OIDC
bucket name and region from the OIDC secret. Those details are already
explicitly provided by the `--oidc-storage-provider-s3-region` and
`--oidc-storage-provider-s3-bucket-name` flags, and reading them from the secret
is both unnecessary and imposes a weird contract on end users bringing their own
secret (who would have to repeat the bucket name and region as "special" keys in
their credentials secret).

Finally, this commit takes the `kube-system/oidc-storage-provider-s3-config`
configmap out of the install output, as this resource is now published
automatically by the hypershift-operator.
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 1, 2022

@ironcladlou: all tests passed!

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 1, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Feb 1, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alvaroaleman, ironcladlou

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [alvaroaleman,ironcladlou]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit 8db88ce into openshift:main Feb 1, 2022
nirarg added a commit to nirarg/hypershift that referenced this pull request Feb 2, 2022
…derS3Secret exists

The code added in PR openshift#949 cause to  command to fail to all not AWS platform
The code try to access to OIDCStorageProviderS3Secret param which initialized only in AWS platform
(initialized only if user add oidc-storage-provider-s3-credentials flag)
Therefore, in order to fix this, validate OIDCStorageProviderS3Secret not empty before the create
nirarg added a commit to nirarg/hypershift that referenced this pull request Feb 2, 2022
…derS3Secret exists

The code added in PR openshift#949 cause to  command to fail to all not AWS platform
The code try to access to OIDCStorageProviderS3Secret param which initialized only in AWS platform
(initialized only if user add oidc-storage-provider-s3-credentials flag)
Therefore, in order to fix this, validate OIDCStorageProviderS3Secret not empty before the create
hardys pushed a commit to hardys/hypershift that referenced this pull request Feb 3, 2022
…derS3Secret exists

The code added in PR openshift#949 cause to  command to fail to all not AWS platform
The code try to access to OIDCStorageProviderS3Secret param which initialized only in AWS platform
(initialized only if user add oidc-storage-provider-s3-credentials flag)
Therefore, in order to fix this, validate OIDCStorageProviderS3Secret not empty before the create
relyt0925 added a commit to relyt0925/hypershift that referenced this pull request Feb 4, 2022
A previous pr regressed the standard behavior where oidc mounts/config were not added to the hypershift operator unless specified. PR is: openshift#949. This PR returns to the old behavior and adds unit tests to ensure this path is not broken in the future
relyt0925 added a commit to relyt0925/hypershift that referenced this pull request Feb 4, 2022
A previous pr regressed the standard behavior where oidc mounts/config were not added to the hypershift operator unless specified. PR is: openshift#949. This PR returns to the old behavior and adds unit tests to ensure this path is not broken in the future
relyt0925 added a commit to relyt0925/hypershift that referenced this pull request Feb 4, 2022
A previous pr regressed the standard behavior where oidc mounts/config were not added to the hypershift operator unless specified. PR is: openshift#949. This PR returns to the old behavior and adds unit tests to ensure this path is not broken in the future
relyt0925 added a commit to relyt0925/hypershift that referenced this pull request Feb 4, 2022
A previous pr regressed the standard behavior where oidc mounts/config were not added to the hypershift operator unless specified. PR is: openshift#949. This PR returns to the old behavior and adds unit tests to ensure this path is not broken in the future
relyt0925 added a commit to relyt0925/hypershift that referenced this pull request Feb 4, 2022
A previous pr regressed the standard behavior where oidc mounts/config were not added to the hypershift operator unless specified. PR is: openshift#949. This PR returns to the old behavior and adds unit tests to ensure this path is not broken in the future
mkumatag pushed a commit to mkumatag/hypershift that referenced this pull request Feb 9, 2022
A previous pr regressed the standard behavior where oidc mounts/config were not added to the hypershift operator unless specified. PR is: openshift#949. This PR returns to the old behavior and adds unit tests to ensure this path is not broken in the future
geoberle added a commit to geoberle/hypershift that referenced this pull request Apr 27, 2022
**What this PR does / why we need it**:

Add `hypershift install` options `--aws-private-secret` and `--aws-private-secret-key` to specify an existing Kubernetes secret and key name for the AWS shared credentials file. The existing `--aws-private-creds` flag (which creates the secret on the user's behalf) is unaffected. These options are helpful in scenarios when the secret is provided by other means. Also, when the installation template is rendered to a file, it should not contain secrets.

The template generated by `hypershift install render --template` was changed to provide parameters for those CLI options, namely `AWS_PRIVATE_REGION`, `AWS_PRIVATE_CREDS_SECRET` and `AWS_PRIVATE_CREDS_SECRET_KEY`.

A similar change has been implemented for the OIDC bucket credentials secret in openshift#949

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle added a commit to geoberle/hypershift that referenced this pull request Apr 27, 2022
**What this PR does / why we need it**:

Add `hypershift install` options `--aws-private-secret` and `--aws-private-secret-key` to specify an existing Kubernetes secret and key name for the AWS shared credentials file. The existing `--aws-private-creds` flag (which creates the secret on the user's behalf) is unaffected. These options are helpful in scenarios when the secret is provided by other means. Also, when the installation template is rendered to a file, it should not contain secrets.

The template generated by `hypershift install render --template` was changed to provide parameters for those CLI options, namely `AWS_PRIVATE_REGION`, `AWS_PRIVATE_CREDS_SECRET` and `AWS_PRIVATE_CREDS_SECRET_KEY`.

A similar change has been implemented for the OIDC bucket credentials secret in openshift#949

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle added a commit to geoberle/hypershift that referenced this pull request Apr 27, 2022
**What this PR does / why we need it**:

Add `hypershift install` options `--aws-private-secret` and `--aws-private-secret-key` to specify an existing Kubernetes secret and key name for the AWS shared credentials file. The existing `--aws-private-creds` flag (which creates the secret on the user's behalf) is unaffected. These options are helpful in scenarios when the secret is provided by other means. Also, when the installation template is rendered to a file, it should not contain secrets.

The template generated by `hypershift install render --template` was changed to provide parameters for those CLI options, namely `AWS_PRIVATE_REGION`, `AWS_PRIVATE_CREDS_SECRET` and `AWS_PRIVATE_CREDS_SECRET_KEY`.

A similar change has been implemented for the OIDC bucket credentials secret in openshift#949

Part of https://issues.redhat.com/browse/HOSTEDCP-306

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
geoberle added a commit to geoberle/hypershift that referenced this pull request Apr 27, 2022
**What this PR does / why we need it**:

Add `hypershift install` options `--aws-private-secret` and `--aws-private-secret-key` to specify an existing Kubernetes secret and key name for the AWS shared credentials file. The existing `--aws-private-creds` flag (which creates the secret on the user's behalf) is unaffected. These options are helpful in scenarios when the secret is provided by other means. Also, when the installation template is rendered to a file, it should not contain secrets.

The template generated by `hypershift install render --template` was changed to provide parameters for those CLI options, namely `AWS_PRIVATE_REGION`, `AWS_PRIVATE_CREDS_SECRET` and `AWS_PRIVATE_CREDS_SECRET_KEY`.

A similar change has been implemented for the OIDC bucket credentials secret in openshift#949

Part of https://issues.redhat.com/browse/HOSTEDCP-415

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
openshift-merge-robot pushed a commit that referenced this pull request Apr 29, 2022
* install: enable user provided AWS credentials secret

**What this PR does / why we need it**:

Add `hypershift install` options `--aws-private-secret` and `--aws-private-secret-key` to specify an existing Kubernetes secret and key name for the AWS shared credentials file. The existing `--aws-private-creds` flag (which creates the secret on the user's behalf) is unaffected. These options are helpful in scenarios when the secret is provided by other means. Also, when the installation template is rendered to a file, it should not contain secrets.

The template generated by `hypershift install render --template` was changed to provide parameters for those CLI options, namely `AWS_PRIVATE_REGION`, `AWS_PRIVATE_CREDS_SECRET` and `AWS_PRIVATE_CREDS_SECRET_KEY`.

A similar change has been implemented for the OIDC bucket credentials secret in #949

Part of https://issues.redhat.com/browse/HOSTEDCP-415

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>

* tests

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>

* split asset test for oidc and aws private creds

Signed-off-by: Gerd Oberlechner <goberlec@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants