Skip to content
This repository has been archived by the owner on Nov 7, 2018. It is now read-only.

Move to cloud.gov in govcloud #838

Closed
wants to merge 10 commits into from
Closed

Conversation

jcscottiii
Copy link

  • Use RDS instance

@jcscottiii
Copy link
Author

cc: @erik-burgess @alain-hoang

@digiphile
Copy link

digiphile commented Mar 1, 2017 via email

@jcscottiii jcscottiii changed the title Move to govcloud [wip] Move to cloud.gov in govcloud [wip] Mar 1, 2017
@jcscottiii
Copy link
Author

@digiphile sorry for the confusion. currently it runs on the cloud.gov platform that resides in AWS E/W. that cloud.gov environment is going away. cloud.gov has a new FedRAMP'ed environment in AWS GovCloud. This PR is just some small changes to make so it will work there eventually.

@jcscottiii jcscottiii changed the title Move to cloud.gov in govcloud [wip] Move to cloud.gov in govcloud Mar 2, 2017
@alain-hoang
Copy link

The changes associated with this pull request addresses compatibility modifications to adapt open.foia.gov for the new cloud.gov that has been FedRAMPed. A note for the person(s) who can perform merge (and deploy) these changes include:

  • Updated the meta-information in the dependencies on required software libraries to run with versions compatible with open.foia.gov
  • Updated the hosts information so that it is compatible with running in the updated cloud.gov environment
  • Removed dependency on unnecessary credentials for accessing object storage assets (S3) in AWS
  • Updated documentation in README for new FedRAMPED environment
    • Included clarifications for setup, running, and deployment of this application
  • Updated the Python runtime to a more compatible version with Cloud Foundry
  • Updated location for database to point to new location in new FedRAMPed environment

While the main issue is that the pull request for these modifications is that someone with the necessary permissions can perform the merge, it is also recommended that someone with access should deploy the application post merge. This will help verify that the master branch (post merge) for this project is in sync with application in cloud.gov.

Copy link
Contributor

@cmc333333 cmc333333 left a comment

Choose a reason for hiding this comment

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

Mostly efficiency comments, but I would like to confirm that we need to remove the AWS env vars from the local settings before merging.

env = AppEnv()
cf_foia_db = env.get_service(name=re.compile('foia-db'))
if cf_foia_db:
database_url = cf_foia_db.credentials['uri']
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think lines 9-12 are needed -- cloud.gov will inject a DATABASE_URL env var. Using an env var directly (rather than jumping through cfenv) reduces coupling.


# See env.example for an explanation of these settings.

SECRET_KEY = os.getenv("FOIA_SECRET_SESSION_KEY")
DATABASES = {'default': dj_database_url.parse(os.getenv("DATABASE_URL"))}
DATABASES = {'default': dj_database_url.parse(database_url)}
Copy link
Contributor

Choose a reason for hiding this comment

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

Heads up, this can be dj_database_url.config(), which will read from os.getenv['DATABASE_URL'].

SHOW_WEBFORM = (os.getenv("FOIA_SHOW_WEBFORM") == "true")
ANALYTICS_ID = os.getenv("FOIA_ANALYTICS_ID")

AWS_ACCESS_KEY_ID = os.getenv('FOIA_AWS_ACCESS_KEY_ID')
AWS_SECRET_ACCESS_KEY = os.getenv('FOIA_AWS_SECRET_ACCESS_KEY')
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem important for running locally. Do we need to delete them?

@@ -3,7 +3,7 @@
# Some possible hosts
ALLOWED_HOSTS = [
'open.foia.gov', 'foia-a.cf.18f.us', 'foia-b.cf.18f.us',
'openfoia-staging.cf.18f.us'
'openfoia-staging.cf.18f.us', 'foia.app.cloud.gov'
]
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we grab this from cfenv? i.e.

env = AppEnv()

ALLOWED_HOSTS = env.uris

@adborden adborden mentioned this pull request Sep 19, 2017
@jcscottiii jcscottiii closed this Mar 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants