Skip to content
This repository has been archived by the owner on Oct 20, 2023. It is now read-only.

Updating the way Forseti Server Configuration is retrieved from GCS #480

Merged

Conversation

hiloboy0119
Copy link

Moved away from google_storage_object_signed_url as it requires
a local json keyfile and I am deploying using service account
impersonation.

hashicorp/terraform-provider-google#3558

Moved away from `google_storage_object_signed_url` as it requires
a local json keyfile and I am deploying using service account
impersonation.

hashicorp/terraform-provider-google#3558
@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@hiloboy0119
Copy link
Author

@googlebot I signed it!

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Feb 3, 2020
Copy link
Contributor

@gkowalski-google gkowalski-google left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! I assume this change is partly dependent on the Helm PR?

examples/on_gke_end_to_end/main.tf Show resolved Hide resolved
modules/on_gke/main.tf Show resolved Hide resolved
modules/on_gke/main.tf Show resolved Hide resolved
@gkowalski-google
Copy link
Contributor

@hiloboy0119 The lint tests failed for 2 reasons:

  1. Please run make docker_generate_docs to update the docs for the helm_chart_version change
  2. This line failed because it's expected that all expressions have the equal signs line up. See my comment on that particular line, which I believe can be removed anyway.

@gkowalski-google
Copy link
Contributor

@hiloboy0119 Following up on this PR, please look into my comments above.

@gkowalski-google
Copy link
Contributor

@hiloboy0119 Another ping on this PR, do you still wish to move forward with this change?

@gkowalski-google
Copy link
Contributor

@hiloboy0119 Checking in to see if you still want to move forward with this PR.

@gkowalski-google
Copy link
Contributor

Thanks @hiloboy0119 for this PR. We are now hitting the same issue on this PR where I am adding tests for the GKE example. I'm going to merge this change into my branch, so that we can fix the tests and include them in the upcoming release.

@gkowalski-google gkowalski-google changed the base branch from master to feature/add-gke-tests March 12, 2020 16:36
@gkowalski-google gkowalski-google merged commit 5cdc2df into forseti-security:feature/add-gke-tests Mar 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants