Added secret file name/extension to gitignore #3331
Conversation
.gitignore
Outdated
| @@ -27,6 +27,12 @@ | |||
| /*.json | |||
| install/gcp/*.json | |||
|
|
|||
| # Ignore JSON key for service account and not all JSON files. | |||
| service-account-key.json | |||
There was a problem hiding this comment.
Hmmm, why is this file in the repo in the first place? Can we expect this file service-account-key.json to be placed outside of the git repo, rather than ignoring it inside the repo?
There was a problem hiding this comment.
Agree, these service account keys should be stored outside the repo.
There was a problem hiding this comment.
The best solution to implement is likely git-secrets checks: #3315
The internal docs on setting up an integration tests currently has you create this file in the repo directory. That needs to be updated with best practice on keeping this out of the repo path.In addition the file travis-integration.env likely needs to follow this same pattern.
| store sensitive information/secrets in the forseti-security repository, store | ||
| project id in TF_VAR_project_id variable, organization id in TF_VAR_org_id, | ||
| domain in TF_VAR_domain, billing account in TF_VAR_billing_account and | ||
| service account key in SERVICE_ACCOUNT_JSON variable. |
There was a problem hiding this comment.
Is this the best way to use the key? Elsewhere, I have seen the usage of environment variable with terraform:
export GOOGLE_APPLICATION_CREDENTIALS="[PATH]"
For example:
export GOOGLE_APPLICATION_CREDENTIALS="/home/user/Downloads/[FILE_NAME].json"
There was a problem hiding this comment.
This is exactly what I meant by setting environment variables from bash shell. Users/developers can go:
export TF_VAR_project_id=<PROJECT_ID>
There was a problem hiding this comment.
This way secrets need not be stored in the forseti-security repo as the environment variables can be set from the bash shell.
There was a problem hiding this comment.
Currently the tests can only be run on the silver org. Users would need the service account key from this org in order to run the tests locally. I think we should hold off on instructing the community that they can run integration tests until these tests are supported in any environment.
There was a problem hiding this comment.
The command below shows how to run tests from the color domain so that developer can run tests on their local testing without storing secrets in forseti-security repository.
There was a problem hiding this comment.
Sorry, I was not clear. The tests can be run against other orgs but the tests will fail because there are several tests that check for resources that only exist in the silver org, such as: https://github.com/forseti-security/forseti-security/blob/master/integration_tests/tests/forseti/controls/explain.rb#L47. I think we should close this PR and I will work on updating any of these tests to be flexible.
There was a problem hiding this comment.
Inventory and model modules will not be impacted. Tests in Explain that are subject to Silver domain will fail as all the tests were written to run in Silver domain only.
No description provided.