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.

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

Refactor deployment #159

Merged
merged 23 commits into from
Mar 16, 2018
Merged

Refactor deployment #159

merged 23 commits into from
Mar 16, 2018

Conversation

fureigh
Copy link
Contributor

@fureigh fureigh commented Mar 9, 2018

This pull request — incorporating excellent contributions from @cmc333333 — is ☁️.gov 🚀.

When installing modules as part of local development, Composer
adds the files for those modules. This commit updates .gitignore
to exclude those files from the repository so we can manage those
changes more efficiently.
This repository has essentially been based in an existing `/web/`
directory. Moving most of its files to a new subdirectory will
allow us to include the files that have been one level above this.
This is part of reorganizing these files within a `/web/` directory.
Drawn from github.com/18F/cf-ex-drupal/. h/t @pburkholder
@fureigh fureigh self-assigned this Mar 9, 2018
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.

Glad to see this coming along! We didn't need to change the composer lock file at all?

# Ignore configuration files that may contain sensitive information.
sites/*/settings*.php
sites/*/*settings*.php*
Copy link
Contributor

Choose a reason for hiding this comment

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

Something to keep an eye on: we may want to include these settings files in the future, but keep all the sensitive info in env vars (via user-provided services). File-based configs don't play too well with the 12-factor app design.

@@ -0,0 +1,3 @@
---
packages:
- mysql-client
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like we're just using the PHP buildpack now -- is this file still needed?

bootstrap.sh Outdated
@@ -0,0 +1,38 @@
#!/bin/bash
set -euxo pipefail
Copy link
Contributor

Choose a reason for hiding this comment

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

Something else to keep an eye on: we probably don't want to echo every command (notably, those that include sensitive env vars), lest they be caught in the CI logs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, thank you!

@@ -0,0 +1,29 @@
# Ignore directories generated by Composer
Copy link
Contributor

Choose a reason for hiding this comment

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

For later: there's a deployment consideration here -- we could include these dirs in the cf deployment (i.e. vendor our dependencies). That'd ensure consistency and slightly speed up restages at the cost of high likelihood of error (e.g. sending up something compiled for OSX).

bootstrap.sh Outdated
drush --root=$HOME/web core-status bootstrap | grep -q "Successful" || bootstrap

drush --root=$HOME/web pm-info flysystem_s3 --fields=Status | grep -q enabled ||
drush --root=$HOME/web pm-enable --yes flysystem_s3
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat! I think we'll need to insert the config-loading code around here.

I didn't see any of the ".bp-config" stuff which is what would pull this script in.

Drawn from github.com/18F/cf-ex-drupal/
Theoretically this precludes the addition of custom modules. May
want to scale back to /contrib at some point.
@OpenGlobe OpenGlobe added this to Backlog (ordered by priority) in Project Tracking Board Mar 14, 2018
@cmc333333 cmc333333 mentioned this pull request Mar 14, 2018
7 tasks
Brightcove API credentials have been redacted.
CM Lubinski and others added 14 commits March 16, 2018 08:40
This doesn't include multiple instances and only covers the demo app, but
should be enough to get us started.
In an effort to closer align with the cloud.gov Drupal example, this adds a
handful of dependencies. It looks like the previous code was also built with a
generator, so this shouldn't be a big issue. All of the previous Drupal
modules should still be present.

Notably, however, this *does* declare that we're using PHP 7.1.
These are largely auto-generated, but we don't want to recreate them every
time an instance restarts, so we'll commit them to the repository.

The exception is the settings.cf.php file, which configures the database
connection, flysystem, and the hash salt based on values injected by cloud.gov
services. The values can be missing (e.g. if running locally), and a local
settings file can be used instead.
This pulls in more configuration from the cloud.gov example. It uses their
bootstrap.sh as a guide, but ends up rewriting it to leak less information and
to make use of our settings files.
It's not used and now misleading.
The uuid isn't sensitive, so we're safe to include it (it might be referenced
by other configs in the future). This also removes the nsf-brightcove module,
which we don't have anymore.
These are needed for the current config import. We'll likely remove some of
these in the future.
The "standard" install profile adds a "shortcut_set" that conflicts with our
configs, so we need to remove it. Similarly, we need to indicate that our
Drupal install is downstream from a set of configurations (by giving it the
same UUID as those configs).
This will effectively duplicate the latest config changes. It also clears the
cache, in case it'd have been affected by code/theme changes.
We'll fetch these from the "secrets" service and set them via drupal
config:override. Note that this sends its output to /dev/null, lest the
sensitive data become part of the app longs.
Edits for cloud.gov configuration
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.

Lots of changes, but we paired through them. Excited to see this get in.

@fureigh fureigh changed the title [WIP] Refactor deployment Refactor deployment Mar 16, 2018
@fureigh fureigh merged commit 8d90aa2 into master Mar 16, 2018
@fureigh fureigh deleted the WIP-cloud-gov-deployment branch March 16, 2018 21:39
fureigh added a commit that referenced this pull request Mar 19, 2018
The sites/default directory now lives inside `web`. This commit is
follow-up on #159 and #167.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Project Tracking Board
Backlog (ordered by priority)
Development

Successfully merging this pull request may close these issues.

None yet

2 participants