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

OADP-1225 Add configs to support Google WIF Authentication in OADP Operator #904

Closed
wants to merge 2 commits into from

Conversation

kaovilai
Copy link
Member

@kaovilai kaovilai commented Feb 15, 2023

OADP-1225
We want to enable keyless cloud providers credentials like Google WIF.

  • NoSecret flag for Google WIF support
  • Remove unused velero reconciler

SAAnnotations for Google WIF support can be added to velero SA using oc annotate

Verification steps

  1. make deploy-olm against this branch
  2. install wif using wiki instructions https://github.com/kaovilai/k8s-config-connector/wiki/Using-Config-Connector-on-OpenShift, then configure ConfigConnector using official docs, annotating openshift-adp as the namespace to create resources in.
  3. we would expect annotations in the namespace like so if 2. was followed.
    ❯ oc get ns openshift-adp -oyaml 
    apiVersion: v1
    kind: Namespace
    metadata:
      annotations:
        cnrm.cloud.google.com/project-id: openshift-gce-devel
      name: openshift-adp
  4. create dpa
    •  # TODO:

Notes:

We are using ConfigConnector to create, acquire, and/or delete bucket resources which after configconnector is installed can be done via creating storagebuckets.storage.cnrm.cloud.google.com CustomResource

We create the following

apiVersion: storage.cnrm.cloud.google.com/v1beta1
kind: StorageBucket
metadata:
  name: example
  namespace: openshift-adp
spec:
  resourceID: <bucket-name>

If everything goes well, the configconnector updates this object to something like

apiVersion: storage.cnrm.cloud.google.com/v1beta1
kind: StorageBucket
metadata:
  annotations:
    cnrm.cloud.google.com/management-conflict-prevention-policy: none
    cnrm.cloud.google.com/project-id: openshift-gce-devel
    cnrm.cloud.google.com/state-into-spec: merge
  resourceVersion: '1597993'
  name: example
  uid: 47653c93-5e96-4385-926a-a2ed2ff1fe20
  creationTimestamp: '2023-04-10T02:47:51Z'
  generation: 2
  managedFields:
    - apiVersion: storage.cnrm.cloud.google.com/v1beta1
      fieldsType: FieldsV1
      fieldsV1:
        'f:spec':
          .: {}
          'f:location': {}
          'f:resourceID': {}
      manager: Mozilla
      operation: Update
      time: '2023-04-10T02:47:51Z'
    - apiVersion: storage.cnrm.cloud.google.com/v1beta1
      fieldsType: FieldsV1
      fieldsV1:
        'f:metadata':
          'f:finalizers':
            .: {}
            'v:"cnrm.cloud.google.com/deletion-defender"': {}
            'v:"cnrm.cloud.google.com/finalizer"': {}
        'f:spec':
          'f:publicAccessPrevention': {}
          'f:storageClass': {}
          'f:uniformBucketLevelAccess': {}
      manager: cnrm-controller-manager
      operation: Update
      time: '2023-04-10T02:47:52Z'
    - apiVersion: storage.cnrm.cloud.google.com/v1beta1
      fieldsType: FieldsV1
      fieldsV1:
        'f:status':
          .: {}
          'f:conditions': {}
          'f:observedGeneration': {}
          'f:selfLink': {}
          'f:url': {}
      manager: cnrm-controller-manager
      operation: Update
      subresource: status
      time: '2023-04-10T02:47:52Z'
  namespace: openshift-adp
  finalizers:
    - cnrm.cloud.google.com/finalizer
    - cnrm.cloud.google.com/deletion-defender
spec:
  location: US
  publicAccessPrevention: inherited
  resourceID: <bucket-name>
  storageClass: STANDARD
  uniformBucketLevelAccess: true
status:
  conditions:
    - lastTransitionTime: '2023-04-10T02:47:52Z'
      message: The resource is up to date
      reason: UpToDate
      status: 'True'
      type: Ready
  observedGeneration: 2
  selfLink: 'https://www.googleapis.com/storage/v1/b/<bucket-name>'
  url: 'gs://<bucket-name>'

This confirms configconnector is able to acquire this resource.

Acronyms:

  • GSA: Google Cloud Service Account
  • KSA: Kubernetes Service Account

Configure applications to use Workload identity, step 4. where we will use existing serviceaccounts by creating following resource with intended service accounts to use to access our buckets
https://cloud.google.com/kubernetes-engine/docs/how-to/workload-identity#config-connector

apiVersion: iam.cnrm.cloud.google.com/v1beta1
kind: IAMServiceAccount
metadata:
  name: [GSA_NAME]
spec:
  displayName: [DISPLAY_NAME]

Perhaps this step means we can use a different GSA (team specific) to access buckets than the one used to install configconnector (admin GSA that can impersonate other lower level GSAs).
We need a cluster admin to install ConfigConnector anyways since its a clusterwide operator.

Follow their step 5.: Double check that this serviceaccount [GSA_NAME] has the roles necessary to access the bucket (and to take native snapshots?) resource.

Follow their step 6. Allow the Kubernetes service account to impersonate the IAM service account using ConfigConnector to use existing GSA with our KSA

Follow their step 7. Annotate the Kubernetes service account with the email address of the IAM service account.

Follow their step 8. Update your Pod spec to schedule the workloads on nodes that use Workload Identity and to use the annotated Kubernetes service account.
Note that the nodeSelector may not apply to us, since we are not a gke cluster, and installed configconnector

Follow their step 9. Apply the updated configuration to your cluster:

verify setup

🤞 Use Workload Identity from your code

Existing code that authenticates using the instance metadata server (like code using the Google Cloud client libraries) should work without modification.

TODO:

  • Verify if we can make this work without creating new bucket
  • Check backup work

@kaovilai kaovilai force-pushed the NoSecretFlag-master branch 2 times, most recently from 730827f to 80a950d Compare February 15, 2023 20:26
@kaovilai kaovilai changed the title Add noSecret flag to skip secret check OADP-1182 Add all velero deployment server args to DPA Feb 15, 2023
@kaovilai
Copy link
Member Author

/retest

@kaovilai kaovilai force-pushed the NoSecretFlag-master branch 3 times, most recently from 0edb424 to 0747309 Compare February 17, 2023 20:04
pluginSpecificMap no secret check minor fix, it was not using the short plugin name ie. `gcp` to match but the full long plugin name.

NoSecret test updates
Comment on lines -723 to +576
providerNeedsDefaultCreds[psf.PluginName] = needDefaultCred
providerNeedsDefaultCreds[string(provider)] = needDefaultCred
Copy link
Member Author

Choose a reason for hiding this comment

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

This fixes the provider name used from velero-plugin-for-gcp to gcp which is what functions using value returned by this function expected.

@kaovilai
Copy link
Member Author

/retest

@kaovilai kaovilai changed the title OADP-1182 Add all velero deployment server args to DPA OADP-1225 Add all velero deployment server args to DPA Feb 20, 2023
@kaovilai kaovilai changed the title OADP-1225 Add all velero deployment server args to DPA OADP-1225 Add configs to support Google WIF Authentication in OADP Operator Feb 20, 2023
@kaovilai
Copy link
Member Author

Looks like service account is not managed by OADP Operator. User can annotate them outside DPA.

Velero SA is installed via OLM via the operator bundle.

ReconcileVeleroServiceAccount
ReconcileVeleroCRDs
InstallVeleroCRDs
ReconcileVeleroClusterRoleBinding
@openshift-ci
Copy link

openshift-ci bot commented Feb 22, 2023

@kaovilai: 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.

@kaovilai kaovilai self-assigned this Apr 6, 2023
@kaovilai
Copy link
Member Author

kaovilai commented Apr 6, 2023

Ways to use WIF in clusters

@kaovilai
Copy link
Member Author

@weshayutin
Copy link
Contributor

@kaovilai so perhaps we don't need these changes iiuc? Pending feedback from customer should we hold this PR?

@kaovilai
Copy link
Member Author

yes.
/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 11, 2023
@kaovilai kaovilai added customer_feature triage/needs-information Indicates an issue needs more information in order to work on it. labels Apr 12, 2023
@kaovilai kaovilai closed this Apr 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
customer_feature do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. triage/needs-information Indicates an issue needs more information in order to work on it.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants