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

Refactoring code sample for connecting to CloudSQL databases using Cloud SQL Connector for Java #8794

Open
3 tasks
minherz opened this issue Nov 1, 2023 · 5 comments
Assignees
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. samples Issues that are directly related to samples. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@minherz
Copy link
Contributor

minherz commented Nov 1, 2023

The repo hosts three code samples that demonstrate the use of the Cloud SQL Connector for Java to connect to the managed versions of MySQL, Postgres and SQL Server databases (CloudSQL).
These three samples are implemented as three-tier applications with the frontend running in the browser, backend running on CloudRun and using CloudSQL database as a datalayer.

In its current format these code samples are hard to maintain. They also go out of the line compared to other code samples because of their complexity, lack of region tag association and loosely trackable exposure via code samples browser only.
Additionally these code samples show connectivity practices that are discouraged due their poor security.

The following modifications are recommended to ALL code samples:

  • Source code should be simplified to cover only connectivity part. If the three tier application format is essential, the code samples should be migrated to designated repo.
  • Source code should utilize region tags; The code snippets can be added to the connection options documentations (see overview page for MySql connection as an example)
  • Authentication methods should be revised to embrace a more secure connectivity methods that leverage underlying identity of GCP infrastructure

These recommendations can be implemented in the scope of this issue or designated issues can be created to address each of them separtely.

@minherz minherz added type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. priority: p2 Moderately-important priority. Fix may not be included in next release. triage me I really want to be triaged. labels Nov 1, 2023
@product-auto-label product-auto-label bot added the samples Issues that are directly related to samples. label Nov 1, 2023
@minherz minherz assigned enocom and unassigned Sita04 Nov 1, 2023
@minherz
Copy link
Contributor Author

minherz commented Nov 1, 2023

@enocom can you please triage this? Does the use of Cloud SQL Connector for Java is different for different DB instance types hosted by CloudSQL? Do we need the connector when we run on GCP (e.g. on Cloud Run) in the first place or the connector is useful when running workloads out of GCP?

Thank you

@enocom enocom removed the triage me I really want to be triaged. label Nov 1, 2023
@enocom
Copy link
Member

enocom commented Nov 1, 2023

Additionally these code samples show connectivity practices that are discouraged due their poor security.

Would you elaborate on that point? What makes these samples have poor security?

Does the use of Cloud SQL Connector for Java is different for different DB instance types hosted by CloudSQL?

There are minor differences between MySQL and Postgres, and SQL Server. If we scope the samples down to just creating a connection (or DataSource more specifically), then we could consolidate all three into one.

Do we need the connector when we run on GCP (e.g. on Cloud Run) in the first place or the connector is useful when running workloads out of GCP?

The Connector is useful across environments. In general, the most important part of these samples are the places we have region tags: 1, 2. In my opinion, if samples are about demonstrating a single connection, then we don't need all the application-related code.

@minherz
Copy link
Contributor Author

minherz commented Nov 1, 2023

Thank you for the explanation. The code sample guidelines require one code snippet per file. Depending on documentation and implementation we can check if this sample can be consolidating into a single file for all three database types. We can include consolidation of the region tags into this issue as a stand-alone topic.

Let me know if you will need assistance in providing code samples in other languages because I see these region tags referenced only Java samples for now.

Regarding your question:

Additionally these code samples show connectivity practices that are discouraged due their poor security.

Would you elaborate on that point? What makes these samples have poor security?

The readme file (the link is for MySql example but it is true for all three) instructs to use SA key and DB credentials explicitly. This practice is long time deprecated in favor of more secure approach that does not require explicitly defining credentials in the environment. Well, you will need some form of authentication if you instruct to run the code sample on prem but this code sample is expected to run on GCP (specifically on Cloud Run). Same way, DB credentials can be loaded from secret manager instead of being used directly. If they do, we should emphasis both in the documentation and in code that this is not secure way and reference to the recommended way.

@enocom
Copy link
Member

enocom commented Nov 2, 2023

Ah I see. Yes, the README is a mess. Shall we just delete them?

@jackwotherspoon
Copy link
Contributor

Ah I see. Yes, the README is a mess. Shall we just delete them?

This might be an issue on some our other language samples as well. We should avoid mentioning downloading SA keys. I don't think removing the READMEs is the optimal solution though. I can fold README updates into my samples work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p2 Moderately-important priority. Fix may not be included in next release. samples Issues that are directly related to samples. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

6 participants
@enocom @minherz @Sita04 @jackwotherspoon and others