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

Support GCP CloudSQL proxy #6606

Merged
merged 5 commits into from
Nov 9, 2021
Merged

Support GCP CloudSQL proxy #6606

merged 5 commits into from
Nov 9, 2021

Conversation

mrsimonemms
Copy link
Contributor

@mrsimonemms mrsimonemms commented Nov 8, 2021

Description

Support GCP CloudSQL. Also change the config so the external DB is provider-agnostic

Related Issue(s)

Fixes #6562

How to test

Set up MySQL DB in GCP

Create secret with following keys:

  • credentials.json - service account to access DB
  • encryptionKeys - use [{"name": "general","version": 1,"primary": true,"material": "4uGh1q8y2DYryJwrVMHs0kWXJlqvHWWt/KJuNi04edI="}]
  • username - whatever you set up in GCP
  • password - ditto

Change config to:

database:
  cloudSQL:
    serviceAccount:
      kind: secret
      name: <secretName>
    instance: <projected>:<region>:<dbName>

Release Notes

Add support for GCP CloudSQL

Documentation

@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #6606 (2662388) into main (e3ab9d7) will decrease coverage by 13.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #6606       +/-   ##
==========================================
- Coverage   19.04%   6.00%   -13.05%     
==========================================
  Files           2      12       +10     
  Lines         168    1116      +948     
==========================================
+ Hits           32      67       +35     
- Misses        134    1048      +914     
+ Partials        2       1        -1     
Flag Coverage Δ
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
installer-raw-app 6.00% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
installer/pkg/common/common.go 4.74% <0.00%> (ø)
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go
installer/pkg/components/ws-manager/deployment.go 0.00% <0.00%> (ø)
...components/ws-manager/unpriviledged-rolebinding.go 0.00% <0.00%> (ø)
installer/pkg/common/objects.go 0.00% <0.00%> (ø)
installer/pkg/components/ws-manager/role.go 0.00% <0.00%> (ø)
installer/pkg/common/display.go 0.00% <0.00%> (ø)
installer/pkg/common/render.go 0.00% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3ab9d7...2662388. Read the comment docs.

@mrsimonemms mrsimonemms changed the title GKE external dependencies Support GCP CloudSQL proxy Nov 9, 2021
Copy link
Contributor

@csweichel csweichel left a comment

Choose a reason for hiding this comment

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

It would also be cool to have a secret validation in place akin to the certificates check.

That said, both comments in this PR can come in later and don't need to be part of this PR. I'd be happy to merge this way (just remove the hold), or add to this PR.

Did not actually test change, but code lgtm.

/hold
/lgtm


-- must be idempotent

CREATE DATABASE IF NOT EXISTS `gitpod-sessions` CHARSET utf8mb4;
Copy link
Contributor

Choose a reason for hiding this comment

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

We could - not necessarily in this PR - move this script (these scripts) to something like pkg/components/database/init

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Possible, but the init scripts are subtly different.

@roboquat
Copy link
Contributor

roboquat commented Nov 9, 2021

LGTM label has been added.

Git tree hash: ed4a33285ff24abfadf954dd2a0c7b267ed75a55

@roboquat
Copy link
Contributor

roboquat commented Nov 9, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: csweichel

Associated issue: #6562

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:

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

@mrsimonemms
Copy link
Contributor Author

/hold cancel

@roboquat roboquat merged commit d2e9f27 into main Nov 9, 2021
@roboquat roboquat deleted the sje/installer-gke-external branch November 9, 2021 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support GCP CloudSQL
3 participants