Update ansible and Dockerfile for tbb #3592

kushaldas · 2018-06-28T07:45:58Z

Status

Ready for review.

Description of Changes

Fixes #3485

Changes proposed in this pull request:

Tor Browser 7.5.6 and geckodriver v0.17.0

Testing

Tests will fail for now in the whole tbb side.

heartsucker · 2018-06-28T08:18:59Z

install_files/ansible-base/roles/app-test/tasks/install_tbb.yml

+    - apt
+
+- name: install geckodriver
+  shell: |


You should use the unarchive module to un-tar the download.

msheiny

Heyy @kushaldas i see some potential tweaks that need to be added first. It's minor and I'm happy to help but I didn't want to step over your PR. Let me know if you don't mind and I'll start throwing commits here.

msheiny · 2018-06-28T19:11:56Z

securedrop/Dockerfile

@@ -20,6 +20,26 @@ RUN curl -LO https://launchpad.net/~ubuntu-mozilla-security/+archive/ubuntu/ppa/
    dpkg -i firefox*deb && apt-get install -f && \
    paxctl -cm /usr/lib/firefox/firefox

+RUN apt-get install -y wget libasound2 libdbus-glib-1-2 libgtk2.0-0 libfontconfig1 libxrender1


Can you add these components to the other layer? (line 9). It's best practice to keep these things bundled. You could separate by a comment as a visual separator.

msheiny · 2018-06-28T19:15:58Z

securedrop/Dockerfile

@@ -20,6 +20,26 @@ RUN curl -LO https://launchpad.net/~ubuntu-mozilla-security/+archive/ubuntu/ppa/
    dpkg -i firefox*deb && apt-get install -f && \
    paxctl -cm /usr/lib/firefox/firefox

+RUN apt-get install -y wget libasound2 libdbus-glib-1-2 libgtk2.0-0 libfontconfig1 libxrender1
+
+RUN gpg --keyserver=hkp://ipv4.pool.sks-keyservers.net --recv-keys EF6E286DDA85EA2A4BA7DE684E2C6E8793298290 && \


I would honestly prefer to drop this network outreach and add the key to version control and then docker COPY it into the container. We constantly battle with flakiness of keyservers in various projects

msheiny · 2018-06-28T19:16:37Z

securedrop/Dockerfile

+    paxctl -cm /root/.local/tbb/tor-browser_en-US/Browser/firefox && \
+    paxctl -cm /root/.local/tbb/tor-browser_en-US/Browser/plugin-container
+
+ENV GECKODRIVER_CHECKSUM=3154274c050d724eb2f4e8986a58ed37c0138b48304692bf7eeed827a5e82319


Can you make this a build ARG instead? Allow over-riding outside of this file

@msheiny Why do we need to override the checksum? Asking because we have to use a particular version of geckodriver with Tor Browser?

oooooooh okay thats not gonna change? i dunno... i just always try to move any unique strings like this outside of the dockerfile.. but i see your point

kushal mentioned waiting for upstream changes for CI. Im happy with his changes so far to this PR

conorsch · 2018-07-10T00:49:06Z

securedrop/Dockerfile

@@ -20,6 +21,27 @@ RUN curl -LO https://launchpad.net/~ubuntu-mozilla-security/+archive/ubuntu/ppa/
    dpkg -i firefox*deb && apt-get install -f && \
    paxctl -cm /usr/lib/firefox/firefox

+
+COPY ./tor_project_public.pub /opt/


Where is this file? Pulling down the branch locally, I can't find it anywhere. Was it omitted from an add? Recall that we already have the tor signing key at install_files/ansible-base/roles/tor-hidden-services/files/tor-signing-key.pub, although we'll have to consider the build context for the image and whether that's accessible here.

Uh, yes, I added that in the tbb_in_dev branch (after I figured that I am missing that) :(

conorsch

Requesting clarification on the Tor apt pubkey location (appears to be missing). Once that's resolved, these changes can certainly be added to the target branch (which is not develop) via merge.

conorsch · 2018-07-10T00:52:50Z

kushal mentioned waiting for upstream changes for CI.

@msheiny Can you elaborate a bit on this? Unclear to me why we'd wait to merge, particularly since this is landing in a feature branch, not develop.

Im happy with his changes so far to this PR

I concur! Modulo a single clarification on the tor apt pubkey (see requested changes above).

redshiftzero · 2018-07-11T18:37:09Z

In sprint planning today, there was a lot of confusion about what the base branch is here, which branch is the right/current one, and what the workflow here is, so I'm going to write out what I think the workflow should be (following the approach we took for the alembic and journalist GUI feature branch work).

tbb-0.9.0 is the base feature branch for the functional testing work. It is not merged into develop until all tickets related to the feature are completed.
PRs implementing tickets for the feature should be proposed here for merge into tbb-0.9.0 as they are completed. There should be one PR per ticket. If the PR is from a fork, the (checked by default) checkbox indicating that maintainers should be allowed to push commits is enabled, this way others can help the PR along if needed.
If two tickets for a given feature are dependent on one another, and the (first) dependent ticket is not yet merged, work should proceed regardless, by pulling the dependent commits into the branch where work is being done for the second ticket. PRs should be done as the work for a ticket is ready for review. The PR author should indicate in the PR description that the PR should not be merged until the PR for the dependent ticket(s) is merged.
Once all tickets for the feature are implemented, a PR is then proposed for final merge of tbb-0.9.0 into develop.

Does that seem reasonable @kushaldas @msheiny @conorsch? Please let me know if there is a particular issue with this workflow and we can revise it.

kushaldas · 2018-07-12T02:58:44Z

@conorsch I cherry-picked the commit which adds the missing tor public key. Rest of the branches I am working on are dependent on this branch. This is ready to be merged in the tbb-0.9.0 branch.

msheiny · 2018-07-12T16:43:07Z

@conorsch can you take another pass here? I dismissed your review since @kushaldas addressed your issue but i wanted to wait to see if you had additional concerns.

kushal addressed original pub key issue

conorsch

LGTM! Dev container builds without error for me now.

Update ansible and Dockerfile for tbb

kushaldas added 2 commits June 28, 2018 13:10

Updates the ansible files for tbb

b85fea5

Updates to the Dockerfile for tbb

26f809a

kushaldas requested a review from conorsch as a code owner June 28, 2018 07:45

kushaldas requested a review from a user June 28, 2018 07:45

kushaldas requested review from heartsucker, msheiny and redshiftzero as code owners June 28, 2018 07:45

kushaldas changed the title ~~Update ansible for tbb~~ Update ansible and Dockerfile for tbb Jun 28, 2018

heartsucker previously requested changes Jun 28, 2018

View reviewed changes

redshiftzero mentioned this pull request Jun 28, 2018

Install Tor browser and geckodriver #3500

Closed

4 tasks

msheiny previously requested changes Jun 28, 2018

View reviewed changes

Updates as suggested in the PR review for ansible and dockerfile

edfbcdf

conorsch reviewed Jul 10, 2018

View reviewed changes

conorsch previously requested changes Jul 10, 2018

View reviewed changes

conorsch mentioned this pull request Jul 10, 2018

[functional testing] Determine how to handle users when testing external server #3488

Closed

Missing tor key

318d8bf

kushaldas mentioned this pull request Jul 12, 2018

Tests against a local instance in the container #3628

Merged

5 tasks

msheiny approved these changes Jul 12, 2018

View reviewed changes

conorsch self-requested a review July 12, 2018 16:57

conorsch approved these changes Jul 12, 2018

View reviewed changes

conorsch merged commit badd904 into freedomofpress:tbb-0.9.0 Jul 12, 2018

redshiftzero mentioned this pull request Jul 13, 2018

[functional testing] Install latest Tor Browser, geckodriver in dev + test #3485

Closed

redshiftzero pushed a commit that referenced this pull request Jul 18, 2018

Merge pull request #3592 from kushaldas/update_ansible_for_tbb

955053d

Update ansible and Dockerfile for tbb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update ansible and Dockerfile for tbb #3592

Update ansible and Dockerfile for tbb #3592

kushaldas commented Jun 28, 2018 •

edited

Loading

heartsucker Jun 28, 2018

msheiny left a comment

msheiny Jun 28, 2018

msheiny Jun 28, 2018

msheiny Jun 28, 2018

kushaldas Jun 29, 2018 •

edited

Loading

msheiny Jun 29, 2018

conorsch Jul 10, 2018

kushaldas Jul 10, 2018

conorsch left a comment

conorsch commented Jul 10, 2018

redshiftzero commented Jul 11, 2018

kushaldas commented Jul 12, 2018

msheiny commented Jul 12, 2018

conorsch left a comment

Update ansible and Dockerfile for tbb #3592

Update ansible and Dockerfile for tbb #3592

Conversation

kushaldas commented Jun 28, 2018 • edited Loading

Status

Description of Changes

Testing

heartsucker Jun 28, 2018

Choose a reason for hiding this comment

msheiny left a comment

Choose a reason for hiding this comment

msheiny Jun 28, 2018

Choose a reason for hiding this comment

msheiny Jun 28, 2018

Choose a reason for hiding this comment

msheiny Jun 28, 2018

Choose a reason for hiding this comment

kushaldas Jun 29, 2018 • edited Loading

Choose a reason for hiding this comment

msheiny Jun 29, 2018

Choose a reason for hiding this comment

conorsch Jul 10, 2018

Choose a reason for hiding this comment

kushaldas Jul 10, 2018

Choose a reason for hiding this comment

conorsch left a comment

Choose a reason for hiding this comment

conorsch commented Jul 10, 2018

redshiftzero commented Jul 11, 2018

kushaldas commented Jul 12, 2018

msheiny commented Jul 12, 2018

conorsch left a comment

Choose a reason for hiding this comment

kushaldas commented Jun 28, 2018 •

edited

Loading

kushaldas Jun 29, 2018 •

edited

Loading