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.

Sign up for GitHub

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

Jump to bottom

feat: Follow least privilege principal for backup service account #597

Merged
merged 5 commits into from
Apr 29, 2024
Merged

feat: Follow least privilege principal for backup service account #597

merged 5 commits into from
Apr 29, 2024

Conversation

ps-occrp
Copy link
Contributor

Follow least privilege principal for backup service account

@ps-occrp
Copy link
Contributor Author

ps-occrp commented Apr 21, 2024 •
edited
Loading

Based on this roles/cloudsql.viewer role is sufficient for export workflows but for backup workflows roles/cloudsql.editor role needs to be used. In any case admin role on all instances is against best practices.

PS: IMHO GCP should create role dedicated for backup/export operation, if someone know a place where I can report/request it please let me know.

@imrannayer
Copy link
Collaborator

/gcbrun

@ps-occrp
Copy link
Contributor Author

@imrannayer can you paste gcbrun logs?

@imrannayer
Copy link
Collaborator

@ps-occrp

TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185: 
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185: Error: Error waiting for Create Service Networking Connection: Error code 7, message: Required 'compute.globalAddresses.list' permission for 'projects/950554355066'
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185: Help Token: AWUw39Wc57_3oV2J97Ll90vNGw4ZrDQA7nq0XMPl40TadheOshi4NGrNrzRzRaLF6jYF3aqJF1zPtoRltZf7K024h7BJgn8Q0UwX3bNn2TG2VE7R
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185: 
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185:   with google_service_networking_connection.vpc_connection,
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185:   on network.tf line 56, in resource "google_service_networking_connection" "vpc_connection":
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185:   56: resource "google_service_networking_connection" "vpc_connection" {
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z command.go:185: 
TestMsSqlFailoverReplica 2024-04-23T01:46:14Z retry.go:99: Returning due to fatal error: FatalError{Underlying: error while running command: exit status 1; 
Error: Error waiting for Create Service Networking Connection: Error code 7, message: Required 'compute.globalAddresses.list' permission for 'projects/950554355066'
Help Token: AWUw39Wc57_3oV2J97Ll90vNGw4ZrDQA7nq0XMPl40TadheOshi4NGrNrzRzRaLF6jYF3aqJF1zPtoRltZf7K024h7BJgn8Q0UwX3bNn2TG2VE7R

  with google_service_networking_connection.vpc_connection,
  on network.tf line 56, in resource "google_service_networking_connection" "vpc_connection":
  56: resource "google_service_networking_connection" "vpc_connection" {
}
    apply.go:34: 
        	Error Trace:	/builder/home/go/pkg/mod/github.com/gruntwork-io/terratest@v0.46.13/modules/terraform/apply.go:34
        	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.13.2/pkg/tft/terraform.go:517
        	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.13.2/pkg/tft/terraform.go:539
        	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.13.2/pkg/tft/terraform.go:569
        	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.13.2/pkg/utils/stages.go:31
        	            				/builder/home/go/pkg/mod/github.com/!google!cloud!platform/cloud-foundation-toolkit/infra/blueprint-test@v0.13.2/pkg/tft/terraform.go:569
        	Error:      	Received unexpected error:
        	            	FatalError{Underlying: error while running command: exit status 1; 
        	            	Error: Error waiting for Create Service Networking Connection: Error code 7, message: Required 'compute.globalAddresses.list' permission for 'projects/950554355066'
        	            	Help Token: AWUw39Wc57_3oV2J97Ll90vNGw4ZrDQA7nq0XMPl40TadheOshi4NGrNrzRzRaLF6jYF3aqJF1zPtoRltZf7K024h7BJgn8Q0UwX3bNn2TG2VE7R
        	            	
        	            	  with google_service_networking_connection.vpc_connection,
        	            	  on network.tf line 56, in resource "google_service_networking_connection" "vpc_connection":
        	            	  56: resource "google_service_networking_connection" "vpc_connection" {
        	            	}
        	Test:       	TestMsSqlFailoverReplica
2024/04/23 01:46:14 RUN_STAGE env var set to apply
2024/04/23 01:46:14 Skipping stage teardown
--- FAIL: TestMsSqlFailoverReplica (123.24s)

@ps-occrp
Copy link
Contributor Author

This does not look like issue related to my change

@imrannayer
Copy link
Collaborator

/gcbrun

@ps-occrp
Copy link
Contributor Author

@imrannayer can you please merge this?

@imrannayer
Copy link
Collaborator

@ps-occrp will these examples verify the change you made?

https://github.com/terraform-google-modules/terraform-google-sql-db/tree/master/examples/mysql-backup-create-service-account

https://github.com/terraform-google-modules/terraform-google-sql-db/tree/master/examples/postgresql-backup-provided-service-account

@ps-occrp
Copy link
Contributor Author

ps-occrp commented Apr 25, 2024 •
edited
Loading

@imrannayer First example with MySQL will verify it but second one with Postgres will not verify it.

@imrannayer
Copy link
Collaborator

@ps-occrp is it possible to update postgres example so the test can verify it?

@imrannayer imrannayer self-assigned this Apr 25, 2024
@ps-occrp
Copy link
Contributor Author

@imrannayer I can update it but I don't think it makes sense, mysql example is creating service account and using it and in that case this PR is involved. Postgresql example uses existing service account. This provides complete coverage, if I update postgresql example to not use existing service account and create new service account than test coverage will reduce.

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer
Copy link
Collaborator

/gcbrun

@ps-occrp
Copy link
Contributor Author

@imrannayer can this be merged?

@imrannayer
Copy link
Collaborator

/gcbrun

@imrannayer imrannayer merged commit 0f18fd7 into terraform-google-modules:master Apr 29, 2024
4 checks passed
@release-please release-please bot mentioned this pull request Apr 26, 2024
Merged
@ps-occrp ps-occrp deleted the least_privilege branch April 29, 2024 11:34
@Silvest89 Silvest89 mentioned this pull request Jul 15, 2024
Closed
tjespers added a commit to tjespers/terraform-google-sql-db that referenced this pull request Jul 15, 2024
@tjespers
fix: cleanup of old instance backups broken due to missing permission
d4f0e44 
This commit reverts the change to using the cloudsql.editor role
for the backup service account from the cloudsql.admin role. This was
introduced in terraform-google-modules#597 but due to this the deletion of old backups on the
instance now receives a 403 error in the workflow. This is due to the
fact that the cloudsql.editor role lacks the cloudsql.backupRuns.delete
permission.

Closes: terraform-google-modules#617
@tjespers tjespers mentioned this pull request Jul 15, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imrannayer imrannayer imrannayer approved these changes

@isaurabhuttam isaurabhuttam Awaiting requested review from isaurabhuttam isaurabhuttam is a code owner

Assignees

@imrannayer imrannayer

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ps-occrp @imrannayer