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

Add GCP storage authentication #434

Merged
merged 56 commits into from
Oct 14, 2021
Merged

Conversation

pa250194
Copy link
Contributor

@pa250194 pa250194 commented Sep 2, 2021

If applied, this PR will add support for Google Cloud Platform as a storage provider without the need to enable S3 interoperability and use HMAC keys. As stated here https://cloud.google.com/storage/docs/authentication/hmackeys. There is a restriction to a maximum of 5 HMAC keys per service account in GCP. This PR enables the use of Workload Identity which the GCP client automatically handles.

The GCP provider handles authentication in two ways. The first way being that the GCP client library will automatically check for the presence of the GOOGLE_APPLICATION_CREDENTIAL environment variable. If this is not found, the GCP client library will search for the Google Application Credential file in the config directory.

The second way to authenticate is by using a GCP Service Account Secret saved as a Kubernetes secret named service account. An example is as follows.

apiVersion: source.toolkit.fluccd.io/v1beta1
kind: Bucket
metadata:
  name: podinfo
  namespace: gitops-system
spec:
  interval: 5m
  provider: gcp
  bucketName: podinfo
  endpoint: storage.googleapis.com
  region: us-east-1
  timeout: 30s
  secretRef:
    name: gcp-service-account
---
apiVersion: v1
kind: Secret
metadata:
  name: gcp-service-account
  namespace: gitops-system
type: Opaque
data:
  serviceaccount: "ewogICAgInR5cGUiOiAic2VydmljZV9hY2NvdW50IiwKICAgICJwcm9qZWN0X2lkIjogInBvZGluZm8iLAogICAgInByaXZhdGVfa2V5X2lkIjogIjI4cXdnaDNnZGY1aGozZ2I1ZmozZ3N1NXlmZ2gzNGY0NTMyNDU2OGh5MiIsCiAgICAicHJpdmF0ZV9rZXkiOiAiLS0tLS1CRUdJTiBQUklWQVRFIEtFWS0tLS0tXG5Id2V0aGd5MTIzaHVnZ2hoaGJkY3U2MzU2ZGd5amhzdmd2R0ZESFlnY2RqYnZjZGhic3g2M2Ncbjc2dGd5Y2ZlaHVoVkdURllmdzZ0N3lkZ3lWZ3lkaGV5aHVnZ3ljdWhland5NnQzNWZ0aHl1aGVndmNldGZcblRGVUhHVHlnZ2h1Ymh4ZTY1eWd0NnRneWVkZ3kzMjZodWN5dnN1aGJoY3Zjc2poY3NqaGNzdmdkdEhGQ0dpXG5IY3llNnR5eWczZ2Z5dWhjaGNzYmh5Z2NpamRiaHl5VEY2NnR1aGNldnVoZGNiaHVoaHZmdGN1aGJoM3VoN3Q2eVxuZ2d2ZnRVSGJoNnQ1cmZ0aGh1R1ZSdGZqaGJmY3JkNXI2N3l1aHV2Z0ZUWWpndnRmeWdoYmZjZHJoeWpoYmZjdGZkZnlodmZnXG50Z3ZnZ3RmeWdodmZ0NnR1Z3ZURjVyNjZ0dWpoZ3ZmcnR5aGhnZmN0Nnk3eXRmcjVjdHZnaGJoaHZ0Z2hoanZjdHRmeWNmXG5mZnhmZ2hqYnZnY2d5dDY3dWpiZ3ZjdGZ5aFZDN3VodmdjeWp2aGhqdnl1amNcbmNnZ2hndmdjZmhnZzc2NTQ1NHRjZnRoaGdmdHloaHZ2eXZ2ZmZnZnJ5eXU3N3JlcmVkc3dmdGhoZ2ZjZnR5Y2ZkcnR0ZmhmL1xuLS0tLS1FTkQgUFJJVkFURSBLRVktLS0tLVxuIiwKICAgICJjbGllbnRfZW1haWwiOiAidGVzdEBwb2RpbmZvLmlhbS5nc2VydmljZWFjY291bnQuY29tIiwKICAgICJjbGllbnRfaWQiOiAiMzI2NTc2MzQ2Nzg3NjI1MzY3NDYiLAogICAgImF1dGhfdXJpIjogImh0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbS9vL29hdXRoMi9hdXRoIiwKICAgICJ0b2tlbl91cmkiOiAiaHR0cHM6Ly9vYXV0aDIuZ29vZ2xlYXBpcy5jb20vdG9rZW4iLAogICAgImF1dGhfcHJvdmlkZXJfeDUwOV9jZXJ0X3VybCI6ICJodHRwczovL3d3dy5nb29nbGVhcGlzLmNvbS9vYXV0aDIvdjEvY2VydHMiLAogICAgImNsaWVudF94NTA5X2NlcnRfdXJsIjogImh0dHBzOi8vd3d3Lmdvb2dsZWFwaXMuY29tL3JvYm90L3YxL21ldGFkYXRhL3g1MDkvdGVzdCU0MHBvZGluZm8uaWFtLmdzZXJ2aWNlYWNjb3VudC5jb20iCn0="

The service account secret is a base 64 encoded string of the GCP service account json file which is in the form:

  {
    "type": "service_account",
    "project_id": "podinfo",
    "private_key_id": "28qwgh3gdf5hj3gb5fj3gsu5yfgh34f45324568hy2",
    "private_key": "-----BEGIN PRIVATE KEY-----\nHwethgy123hugghhhbdcu6356dgyjhsvgvGFDHYgcdjbvcdhbsx63c\n76tgycfehuhVGTFYfw6t7ydgyVgydheyhuggycuhejwy6t35fthyuhegvcetf\nTFUHGTygghubhxe65ygt6tgyedgy326hucyvsuhbhcvcsjhcsjhcsvgdtHFCGi\nHcye6tyyg3gfyuhchcsbhygcijdbhyyTF66tuhcevuhdcbhuhhvftcuhbh3uh7t6y\nggvftUHbh6t5rfthhuGVRtfjhbfcrd5r67yuhuvgFTYjgvtfyghbfcdrhyjhbfctfdfyhvfg\ntgvggtfyghvft6tugvTF5r66tujhgvfrtyhhgfct6y7ytfr5ctvghbhhvtghhjvcttfycf\nffxfghjbvgcgyt67ujbgvctfyhVC7uhvgcyjvhhjvyujc\ncgghgvgcfhgg765454tcfthhgftyhhvvyvvffgfryyu77reredswfthhgfcftycfdrttfhf/\n-----END PRIVATE KEY-----\n",
    "client_email": "test@podinfo.iam.gserviceaccount.com",
    "client_id": "32657634678762536746",
    "auth_uri": "https://accounts.google.com/o/oauth2/auth",
    "token_uri": "https://oauth2.googleapis.com/token",
    "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs",
    "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/test%40podinfo.iam.gserviceaccount.com"
  }

**Note:**There are no breaking changes to the current generic provider which provides support for using GCP storage by
enabling S3 compatible access in your GCP project

…ntity

Added Support for Google Cloud Storage with Workload Identity as Source Provider. This enables the use of GCP without enabling S3 compatible access.

Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
@pa250194 pa250194 marked this pull request as ready for review September 16, 2021 17:33
@squaremo
Copy link
Member

I'm interested to know what led to the GCP client having resumable downloads -- it doesn't seem to be needed by the controller, which always fetches into a fresh temp directory.

@pa250194
Copy link
Contributor Author

Hi @squaremo I was trying to make the GCP client to closely resemble the Minio client that is already implemented which has resumable downloads implemented. But I can remove it as I see what you mean that it is not needed by the controller.

@squaremo
Copy link
Member

I was trying to make the GCP client to closely resemble the Minio client that is already implemented

Ah I see! That makes sense. I think that since it's not used by the controller, and not covered by tests (if I read them right), it's probably better to remove that bit for now in favour of simpler code.

The CI failure looks like a transient problem, let's see what happens on the next push.

Last thing: a belated first-time contributor high five ✋ ! Thank you for putting in time and effort on this.

@pa250194
Copy link
Contributor Author

Thank you! ✋🏾 I will make the changes and push them as soon as possible.

Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
@pa250194
Copy link
Contributor Author

I just removed the resumable downloads and refactored the comments. Any other comments or changes will be appreciated. Thank you

@squaremo squaremo self-assigned this Sep 27, 2021
hiddeco and others added 24 commits October 14, 2021 13:48
To allow building a multi-platform container image using `buildx`.

Various configuration flags allow for fine(r)-grain control over the
build process:

- `BASE_IMG`: FQDN of the base image that should be used, without a
  tag.
- `BASE_TAG: tag of the base image that should be used. Allows checksum
  sum to be included.
- `BUILDX_PLATFORMS`: platforms to target for the final container
  image.
- `BUILDX_ARGS`: additional `docker buildx build` arguments, e.g.
  `--push` to push the result to a (local) image registry.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
To provide a better (contributing) experience to those with Apple
machines, as determining the correct paths there is a bit harder.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This can be useful on machines where libgit2 is installed due to other
applications depending on it, but where the composition of this
installation does not properly work with the controller.

Reason the system version is still preferred, is because this lowers the
barrier for drive-by contributors, as a working set of (Git) dependencies
should only really be required if you are going to perform work in that
domain.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Hidde Beydals <hello@hidde.co>
This moves the `libgit2` compilation to the image, to ensure it
can be build on builders that aren't backed by AMD64.

The image is structured in such a way that e.g. running nightly
builds targeting a different Go version, or targeting a different
OS vendor would be possible in the future via build arguments.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This ensures the Dockerfile used for testing is making use of the
same scratch image to compile `libgit2` as the actual application
image.

In a future iteration we should restructure our GitHub Action
workflows to re-use the application image, saving us an additional
Dockerfile and a duplicate build. Inspiration for this (which makes
use of a local registry for the duration of the build) can be found
at: https://github.com/fluxcd/golang-with-libgit2/blob/main/.github/workflows/build.yaml

Signed-off-by: Hidde Beydals <hello@hidde.co>
As this isn't available on Darwin by default, unlike on most Linux
distributions.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: Stefan Prodan <stefan.prodan@gmail.com>
This commit adds a `ReconcileStrategy` field to the `HelmChart` resource, which
allows defining when a new chart should be packaged and/or published if it
originates from a `Bucket` or `GitRepository` resource.

The two available strategies are:

- `ChartVersion`: creates a new artifact when the version of the Helm chart as
  defined in the `Chart.yaml` from the Source is different from the current
  version.
- `Revision`: creates a new artifact when the revision of the Source is
  different from the current revision.

For the `Revision` strategy, the (checksum part of the) revision of the
artifact the chart originatesfrom is added as SemVer metadata.

A chart from a `GitRepository` with Artifact revision
`main/f0faacd5164a875ebdbd9e3fab778f49c5aadbbc` and a chart with e.g. SemVer
`0.1.0` will be published as `0.1.0+f0faacd5164a875ebdbd9e3fab778f49c5aadbbc`.

A chart from a `Bucket` with Artifact revision
`f0faacd5164a875ebdbd9e3fab778f49c5aadbbc` and a chart with e.g. SemVer `0.1.0`
will be published as `0.1.0+f0faacd5164a875ebdbd9e3fab778f49c5aadbbc`.

Signed-off-by: Dylan Arbour <arbourd@users.noreply.github.com>
Signed-off-by: Hidde Beydals <hello@hidde.co>
The version was accidentally set to an invalid version, causing the
API documentation generation to fail.

Signed-off-by: Hidde Beydals <hello@hidde.co>
This includes a tiny fix for Darwin to ensure the generated `.pc`
file includes the right paths.

Signed-off-by: Hidde Beydals <hello@hidde.co>
Signed-off-by: pa250194 <pa250194@ncr.com>
Added Support for Google Cloud Storage with Workload Identity as
Source Provider. This enables the use of GCP without enabling S3
compatible access.

Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>
Signed-off-by: pa250194 <pa250194@ncr.com>

Added log for GCP provider auth error

Signed-off-by: pa250194 <pa250194@ncr.com>
@pa250194
Copy link
Contributor Author

@pa250194 if you drop commit f62571b (and rebase on top of main), this should be good to go. Reason the log can be dropped is because the err is returned, which is logged as well.

Okay got it. I dropped commit f62571b and rebased to main. Please tell me if there is anything else I need to change. Thank you 🙂

Copy link
Member

@hiddeco hiddeco left a comment

Choose a reason for hiding this comment

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

Thank you for taking care of the last nitpicks @pa250194 🙇 💯

@pa250194
Copy link
Contributor Author

Thank you for taking care of the last nitpicks @pa250194 🙇 💯

You're welcome!

@pa250194
Copy link
Contributor Author

Hi @squaremo I created PR 455 - #455 with the refactor of the bucket controller providing the bucket provider interface. Please tell me if there is anything else I need to change or add. Thank you 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/bucket Bucket related issues and pull requests enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants