Nested Virtualization of Staging Env in CI #3909

msheiny · 2018-10-30T19:11:33Z

Status

Ready for review

Description of Changes

Fixes #3702

Changes proposed in this pull request:

Drops AWS staging logic
Introduces GCE staging logic
Now with 💯 more GRSEC tests (previously there were none)

Testing

How should the reviewer test this PR?

To replicate CI env locally:

Get access to Google service account credentials (talk to @msheiny 😍 )
export that service account as an environment variable called GOOGLE_CREDENTIALS
source ci env file (. devops/gce-nested/ci-env.sh)
Start the environment (devops/gce-nested/gce-start.sh)
Start the test (devops/gce-nested/gce-runner.sh)
Kill the test (devops/gce-nested/gce-stop.sh)

1 GRSEC tests is failing and has been xfailed. Making a new ticket to track fix for that. The problem is reproducible locally and existed before introduction of this PR.

Deployment

Any special considerations for deployment? Consider both:

Only affects CI

msheiny · 2018-10-31T02:24:33Z

ARGG EYE SPOT a FLAKY TEST in https://circleci.com/gh/freedomofpress/securedrop/19160

re: #3553

conorsch

Took a pass through and modified the shell scripts somewhat, mostly for readability. Logic is solid! Flagging that we're blocked on merge of #3913, otherwise tests will fail here. Major asks here before merge are:

Update those docs! https://docs.securedrop.org/en/release-0.10.0/development/testing_continuous_integration.html still talks about AWS and lack of grsec testing—no longer true!
Makefile still contains a bunch of references to the old AWS logic; should be removed
devops/jenkins/TorNightlyPipeline will start to fail post merge if not updated; it still references the old AWS logic, as well

After those changes are up, happy to take another look. Eager to get this one in.

msheiny · 2018-11-03T00:02:07Z

Update those docs! https://docs.securedrop.org/en/release-0.10.0/development/testing_continuous_integration.html still talks about AWS and lack of grsec testing—no longer true!

I'll make those docs changes in a separate PR with a docs-* prefix. Iterating on the various nits that I'm sure will come up when updating documentation is really annoying to do when the feedback loop from the staging tests are so long. Or I'd be willing to start collab in something like etherpad first.

conorsch · 2018-11-03T00:04:38Z

I'll make those docs changes in a separate PR

A PR into this PR would work; that way we can skip the long-running staging logic, and still make sure that the dev docs are accurate, since the CI changes and accompanying docs will land in develop together.

conorsch · 2018-11-26T18:47:56Z

Rebased on top of 36cbb21 (latest develop), to re-run CI, including changes in particular from #3913.

codecov-io · 2018-11-26T19:27:03Z

Codecov Report

Merging #3909 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3909   +/-   ##
========================================
  Coverage    84.62%   84.62%           
========================================
  Files           43       43           
  Lines         2739     2739           
  Branches       296      296           
========================================
  Hits          2318     2318           
  Misses         354      354           
  Partials        67       67

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b4924c...4edcaf4. Read the comment docs.

heartsucker · 2018-11-28T23:22:37Z

molecule/testinfra/staging/common/test_grsecurity.py

@@ -160,8 +143,7 @@ def test_apt_autoremove(Command):
    assert "The following packages will be REMOVED" not in c.stdout


-@pytest.mark.skipif(os.environ.get('FPF_GRSEC', 'true') == "false",
-                    reason="Need to skip in environment w/o grsec")
+@pytest.mark.xfail(strict=True)


Can you comment the reason for xfail-ing this instead of skipping? And if possible, can you narrow the scope of the xfail to the exception we expect to see?

I assume it's #3916, but you're right that we should note that explicitly. While handling the docs update here (as discussed during sprint planning today), I'll clarify. Thanks for flagging, @heartsucker.

This doesn't really confirm to upstream ansible's usage of libcloud.. which has been driving me insane. There also isnt an upstream module for this capability.. so decided to write it myself :) Hooray for "not-invented-here" !!!

Introducing this to get around the wacky limitations of gcloud sdk so that what local users test will perfectly match production

Utilize a gcloud sdk container to run all operations. This is really not necessary for CI BUT in order to provide a uniform testing environment against local development and CI this was necessary. Otherwise if a local developer used their machine loaded gcloud sdk, there is a possiblility of collusion of permissions and things launching with elevated permissions. This is not necessarilly a dangerous situation just one that doesnt adequately mimic CI enough... So tl;dr Lets make CI and local dev CI testing as close as possible

Wanted to get this closer in proximity to all the other CI GCE related tooling (since thats what its applicable to). Made some core changes so that this will be the same script that runs in CI and for local ops developers while testing.

I really didnt want to go this route since molecule adds surrounding tooling fun.. but yeah.. here it is for the sake of striping things down to simplicity.

Remove AWS references and detection code. Flop those over for GCE usage.

These are exactly the types of errors we want to catch in CI!! yay! This is a legitimate failure though - I'm able to reproduce locally. Lets flag as an XFAIL an fix in another PR.

Useful for interpretation and formatting of test results by CircleCI

shellcheck -- lots of ignores. a lot of these errors dont apply. Some of them do but they really seem to be things like double quoting (which ive always found problematic), printf usage, sourcing warnings.

Removed the shellcheck exemptions at the top of the files to get warnings displayed again, then improved based on that output. Reduced the "source" calls from 2 to 1, and in general made the scripts a bit more readable. Sprinkled comments liberally throughout, to bless future maintainers.

conorsch · 2018-11-29T19:16:31Z

Rebased on top of 9b4924c (latest develop), will bolt on docs shortly.

conorsch · 2018-12-01T00:08:47Z

Just noticing that we lost the "Don't run staging CI if branch begins with docs-" logic... if I find more issues like that, may punt to follow-up tickets in order to get this in.

@heartsucker

Rightfully pointed out by @heartsucker during review. Cross-linked to relevant issue. As to why we're using xfail rather than skip, the "strict" setting on xfail will alert us if the tests unexpectedly start passing (ain't that the dream?).

Cleaned up the docs a bit to reference the new script paths, as well as excise outdated info such as "we don't run kernel tests in CI". We do! The Makefile targets aren't necessary, but do provide a bit of convenience to developers. Updated the "ci-go" logic in particular, in order to preserve the "Don't run staging CI if branch name starts with `docs-`" logic.

conorsch · 2018-12-01T00:51:11Z

Resolved the outstanding concerns, except for:

devops/jenkins/TorNightlyPipeline will start to fail post merge if not updated; it still references the old AWS logic, as well

It's true the AWS references should not be there, but in order to resolve, we need override support on the GCE logic to provide custom vars. Probably a wrapper script is best; I'd prefer to observe the Jenkins failures (which are only shown internally, since they run on a nightly basis, and will not break CI on PRs to this repo). @msheiny feel free to chime in here, but I suggest we merge as-is and follow-up on refining the Jenkins logic on the backend.

Waiting to approve until CI passes, since I made changes to the logic as part of final review.

Make sure the environment is destroyed in a second step, and mask errors on that second step. Required since the switch to make targets for CI, as part of PR review on the introduction of the new GCE CI logic.

msheiny · 2018-12-03T17:25:11Z

devops/jenkins/TorNightlyPipeline will start to fail post merge if not updated; it still references the old AWS logic, as well

Just to flag here -- that Jenkins job is no longer being run at all. So it can be deleted and purged from the branch whenever.

conorsch · 2018-12-03T17:28:23Z

Thanks for confirming, @msheiny! When I dug around in the interface, looks like it's been dormant for a while, so I opted to ignore. If it's cleaner, happy to excise in this PR, if it's going to remain a dead codepath for a while.

msheiny · 2018-12-03T17:46:35Z

If it's cleaner, happy to excise in this PR, if it's going to remain a dead codepath for a while.

I have another pending branch in infra to squash the job from jenkins (its still there but the trigger to run it is disabled so it never runs). I can pull an accompanying PR in SD to kill it but i'm fine if you want to purge it here instead.

redshiftzero · 2019-01-15T00:47:05Z

.circleci/config.yml

-
-      - run:
-          name: Run static security testing on source code
-          command: make bandit


was removing these steps (static analysis and checking for CVEs) intentional?

ok if not (I will add them again), just was attempting to make some unrelated CI changes over in xenial-pgp-journalist and noticed this

No, that was not intentional, @redshiftzero — my oversight during review. I'll open an issue to track the re-add, fine if it comes in via your Xenial-related CI changes.

Accidentally removed during: #3909

Accidentally removed during: freedomofpress#3909

Accidentally removed during: #3909

msheiny requested a review from conorsch as a code owner October 30, 2018 19:11

msheiny force-pushed the 3702_NestedVirtCI branch 8 times, most recently from 3671c55 to ac6e936 Compare October 31, 2018 02:04

msheiny force-pushed the 3702_NestedVirtCI branch 2 times, most recently from 082e89c to 1425c0f Compare October 31, 2018 03:48

conorsch mentioned this pull request Nov 2, 2018

Update grsecurity kernels to 4.4.162 #3913

Merged

6 tasks

conorsch suggested changes Nov 2, 2018

View reviewed changes

conorsch mentioned this pull request Nov 2, 2018

PaX flags are unset during install time #3916

Closed

conorsch force-pushed the 3702_NestedVirtCI branch from 469c0c0 to 2ef7bb5 Compare November 2, 2018 03:09

conorsch force-pushed the 3702_NestedVirtCI branch from 2ef7bb5 to 6db41b6 Compare November 26, 2018 18:47

heartsucker reviewed Nov 28, 2018

View reviewed changes

msheiny added 5 commits November 29, 2018 14:14

Add custom GCE image fact collector

9d80387

This doesn't really confirm to upstream ansible's usage of libcloud.. which has been driving me insane. There also isnt an upstream module for this capability.. so decided to write it myself :) Hooray for "not-invented-here" !!!

Add docker builder logic for gcloud container tool

4c96c18

Introducing this to get around the wacky limitations of gcloud sdk so that what local users test will perfectly match production

Rename CI-env source script, expand logic

33da166

Wanted to get this closer in proximity to all the other CI GCE related tooling (since thats what its applicable to). Made some core changes so that this will be the same script that runs in CI and for local ops developers while testing.

Drop gce-nested molecule bits, to pure scripts

14070e2

I really didnt want to go this route since molecule adds surrounding tooling fun.. but yeah.. here it is for the sake of striping things down to simplicity.

msheiny and others added 6 commits November 29, 2018 14:14

Run full test process in CI GCE

49e840f

Swap AWS CircleCI env with GCE

a0eb971

Remove AWS references and detection code. Flop those over for GCE usage.

XFAIL pax flags not being applied - legit failure

61ab928

These are exactly the types of errors we want to catch in CI!! yay! This is a legitimate failure though - I'm able to reproduce locally. Lets flag as an XFAIL an fix in another PR.

Pull back junit test results from GCE env

c46879b

Useful for interpretation and formatting of test results by CircleCI

Feed shellcheck,flake8 linters

e77d7d1

shellcheck -- lots of ignores. a lot of these errors dont apply. Some of them do but they really seem to be things like double quoting (which ive always found problematic), printf usage, sourcing warnings.

conorsch force-pushed the 3702_NestedVirtCI branch from 6db41b6 to 374675f Compare November 29, 2018 19:15

Adds 'reason' for xfail on PaX tests

fd2b5fb

Rightfully pointed out by @heartsucker during review. Cross-linked to relevant issue. As to why we're using xfail rather than skip, the "strict" setting on xfail will alert us if the tests unexpectedly start passing (ain't that the dream?).

conorsch force-pushed the 3702_NestedVirtCI branch from 0955f37 to fd2b5fb Compare December 1, 2018 00:17

Always destroy GCE CI nodes, exit zero

4edcaf4

Make sure the environment is destroyed in a second step, and mask errors on that second step. Required since the switch to make targets for CI, as part of PR review on the introduction of the new GCE CI logic.

conorsch approved these changes Dec 3, 2018

View reviewed changes

conorsch merged commit 6d0d246 into develop Dec 4, 2018

redshiftzero reviewed Jan 15, 2019

View reviewed changes

redshiftzero added a commit that referenced this pull request Jan 15, 2019

ci: Add back static analysis and checking Python deps for CVEs

106fa11

Accidentally removed during: #3909

conorsch mentioned this pull request Jan 15, 2019

Safety & bandit checks not running in CI #4033

Closed

redshiftzero added a commit that referenced this pull request Jan 15, 2019

ci: Add back static analysis and checking Python deps for CVEs

079dc18

Accidentally removed during: #3909

redshiftzero added a commit that referenced this pull request Jan 15, 2019

ci: Add back static analysis and checking Python deps for CVEs

6d3da96

Accidentally removed during: #3909

zenmonkeykstop pushed a commit to zenmonkeykstop/securedrop that referenced this pull request Feb 9, 2019

ci: Add back static analysis and checking Python deps for CVEs

0832e29

Accidentally removed during: freedomofpress#3909

heartsucker deleted the 3702_NestedVirtCI branch February 27, 2019 10:36

kushaldas pushed a commit that referenced this pull request Sep 25, 2019

ci: Add back static analysis and checking Python deps for CVEs

f35e902

Accidentally removed during: #3909

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Nested Virtualization of Staging Env in CI #3909

Nested Virtualization of Staging Env in CI #3909

msheiny commented Oct 30, 2018 •

edited

Loading

msheiny commented Oct 31, 2018

conorsch left a comment

msheiny commented Nov 3, 2018

conorsch commented Nov 3, 2018

conorsch commented Nov 26, 2018

codecov-io commented Nov 26, 2018 •

edited

Loading

heartsucker Nov 28, 2018

conorsch Nov 28, 2018

conorsch commented Nov 29, 2018

conorsch commented Dec 1, 2018

conorsch commented Dec 1, 2018

msheiny commented Dec 3, 2018

conorsch commented Dec 3, 2018

msheiny commented Dec 3, 2018 •

edited

Loading

redshiftzero Jan 15, 2019

conorsch Jan 15, 2019

Nested Virtualization of Staging Env in CI #3909

Nested Virtualization of Staging Env in CI #3909

Conversation

msheiny commented Oct 30, 2018 • edited Loading

Status

Description of Changes

Testing

Deployment

msheiny commented Oct 31, 2018

conorsch left a comment

Choose a reason for hiding this comment

msheiny commented Nov 3, 2018

conorsch commented Nov 3, 2018

conorsch commented Nov 26, 2018

codecov-io commented Nov 26, 2018 • edited Loading

Codecov Report

heartsucker Nov 28, 2018

Choose a reason for hiding this comment

conorsch Nov 28, 2018

Choose a reason for hiding this comment

conorsch commented Nov 29, 2018

conorsch commented Dec 1, 2018

conorsch commented Dec 1, 2018

msheiny commented Dec 3, 2018

conorsch commented Dec 3, 2018

msheiny commented Dec 3, 2018 • edited Loading

redshiftzero Jan 15, 2019

Choose a reason for hiding this comment

conorsch Jan 15, 2019

Choose a reason for hiding this comment

msheiny commented Oct 30, 2018 •

edited

Loading

codecov-io commented Nov 26, 2018 •

edited

Loading

msheiny commented Dec 3, 2018 •

edited

Loading