forseti-security / terraform-google-forseti

A Terraform module for installing Forseti on GCP

Apache License 2.0

132 stars 127 forks source link

delete google-cloud-sdk install from the startup-script #622

Closed e-koma closed 3 years ago

e-koma commented 3 years ago

Summery

Currently, when creating the forseti in GCE with Terraform, the forseti startup-script does not work. In the forseti slack community, forseti newers regularly mention that "I can't install forseti" , and each time I've guided them to #618.

Applying this Pull Request, forseti startup-script will work. many people are having trouble, could you please merge it?

Error

yum install google-cloud-sdk in the forseti startup-script fails

06:11:55 forseti-server-vm-dev startup-script: INFO startup-script: No apt package "google-cloud-sdk", but there is a snap with that name.
06:11:55 forseti-server-vm-dev startup-script: INFO startup-script: Try "snap install google-cloud-sdk"
06:11:55 forseti-server-vm-dev startup-script: INFO startup-script: E: Unable to locate package google-cloud-sdk
06:11:55 forseti-server-vm-dev startup-script: INFO startup-script: Return code 100.
06:11:55 forseti-server-vm-dev startup-script: INFO Finished running startup scripts.

Cause

I guess this is because google-cloud-sdk has been deleted from the ubuntu package. but google-cloud-sdk is already installed on GCE instances by default, so we can simply remove google-cloud-sdk.

618 was auto-closed, but I think this fix is necessary, so I re-created a Pull Request.

gkowalski-google commented 3 years ago

/gcbrun

gkowalski-google commented 3 years ago

Thanks! This change looks good, waiting for the tests to complete.

gkowalski-google commented 3 years ago

A few of the tests failed where the VMs did not start properly, I'll need to look into that to see what happened.

gkowalski-google commented 3 years ago

@e-koma Can you edit this PR to merge the change into master? We would need to create a new minor release once the change is merged in.

e-koma commented 3 years ago

@gkowalski-google

Oh, I see. however, the master branch already contains another method for google-cloud-sdk installation. master branch script seems to work fine, so this Pull Request is not necessary for master branch.

Or, I need to understand the branch history.

if modulerelease522 is the latest version, we need to merge master to modulerelease522 branch.
but If master branch is the latest version, we need to modify the forseti document. https://forsetisecurity.org/docs/latest/setup/install/index.html#create-the-service-account-and-enable-required-apis (I understand modulerelease522 is the latest branch because the forseti docs specify to modulerelease522. )

If you know any history history, could you please let me know?

gkowalski-google commented 3 years ago

@gkowalski-google

Oh, I see. however, the master branch already contains another method for google-cloud-sdk installation. master branch script seems to work fine, so this Pull Request is not necessary for master branch.

Or, I need to understand the branch history.

if modulerelease522 is the latest version, we need to merge master to modulerelease522 branch.

but If master branch is the latest version, we need to modify the forseti document. https://forsetisecurity.org/docs/latest/setup/install/index.html#create-the-service-account-and-enable-required-apis (I understand modulerelease522 is the latest branch because the forseti docs specify to modulerelease522. )

If you know any history history, could you please let me know?

Sorry, you are right! I wasn't sure if this was an issue that existed on the main branch. I will try to spend some time to figure out why the tests are failing.

gkowalski-google commented 3 years ago

I updated the PR to point to a new release branch. I'm working on getting access to the test logs so I can see why these failed.

e-koma commented 3 years ago

thanks 🙏

gkowalski-google commented 3 years ago

/gcbrun

gkowalski-google commented 3 years ago

/gcbrun

e-koma commented 3 years ago

thank you!!