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

Fix docker-dev-shell ruby/php build #2324

Merged
merged 4 commits into from Jul 17, 2020
Merged

Conversation

feelepxyz
Copy link
Contributor

When running docker dev shell it bails adding the keyserver. Adding the
repository seems to work but not sure if we're implicitly allowing any
key to be trusted if the repository gets pawned.

Update comment to say brightbox ruby 2.6 now installs 2.6.6.

When running docker dev shell it bails adding the keyserver. Adding the
repository seems to work but not sure if we're implicitly allowing any
key to be trusted if the repository gets pawned.

Update comment to say brightbox ruby 2.6 now installs 2.6.6.
@feelepxyz
Copy link
Contributor Author

Looks like rubocop has been updated to 0.88 and the new version no longer loads config files from the parent directory possibly because of this change: rubocop/rubocop#8239

.rubocop.yml Show resolved Hide resolved
@@ -41,7 +41,7 @@ namespace :ci do
packages = changed_packages
puts "Running rubocop on: #{packages.join(', ')}"
packages.each do |package|
run_command("cd #{package} && bundle exec rubocop")
run_command("cd #{package} && bundle exec rubocop -c ../.rubocop.yml")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this change broke the previous behaviour of checking for config files in the parent directory rubocop/rubocop#8239

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although not sure this is true after looking at the code a bit closer

@feelepxyz feelepxyz force-pushed the feelepxyz/fix-docker-dev-shell branch 4 times, most recently from a010dd6 to 8e5856a Compare July 16, 2020 16:45
@feelepxyz feelepxyz force-pushed the feelepxyz/fix-docker-dev-shell branch from 2e3e46f to e8a1288 Compare July 17, 2020 09:46
Copy link
Contributor

@thepwagner thepwagner left a comment

Choose a reason for hiding this comment

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

I dig it; pinning the key ID in our code was nice (e.g. could a compromised PPA add a new signing key? no clue I don't use PPAs 🤷). An alternative might be vendoring the keys:

gpg --keyserver keyserver.ubuntu.com --recv-keys 4F4EA0AAE5267A6C
gpg --armor --export 4F4EA0AAE5267A6C  > ondrej.gpg

Then the docker build can COPY in and apt-key add < ondrej.gpg.

👎 Completely overkill given :104 is curl | php, just nerdsniped by trust chains.

@feelepxyz
Copy link
Contributor Author

@jurre want to give this a re-review?

RUN echo "deb http://ppa.launchpad.net/ondrej/php/ubuntu bionic main" >> /etc/apt/sources.list.d/ondrej-php.list \
&& echo "deb-src http://ppa.launchpad.net/ondrej/php/ubuntu bionic main" >> /etc/apt/sources.list.d/ondrej-php.list \
&& apt-key adv --keyserver keyserver.ubuntu.com --recv-keys 4F4EA0AAE5267A6C \
RUN add-apt-repository ppa:ondrej/php \
&& apt-get update \
&& apt-get install -y php7.4 php7.4-cli php7.4-xml php7.4-json php7.4-zip php7.4-mbstring php7.4-intl php7.4-common php7.4-gettext php7.4-curl php7.4-bcmath php7.4-gmp php7.4-imagick php7.4-gd php7.4-redis php7.4-soap php7.4-ldap php7.4-memcached php7.4-sqlite3 php7.4-apcu php7.4-tidy php7.4-mongodb php7.4-zmq php7.4-mysql php7.4-imap php7.4-geoip \
&& curl -sS https://getcomposer.org/installer | php \
Copy link
Contributor Author

@feelepxyz feelepxyz Jul 17, 2020

Choose a reason for hiding this comment

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

Looks like this is a better way https://getcomposer.org/download/

php -r "copy('https://getcomposer.org/installer', 'composer-setup.php');"
php -r "if (hash_file('sha384', 'composer-setup.php') === 'e5325b19b381bfd88ce90a5ddb7823406b2a38cff6bb704b0acc289a09c8128d4a8ce2bbafcd1fcbdc38666422fe2806') { echo 'Installer verified'; } else { echo 'Installer corrupt'; unlink('composer-setup.php'); } echo PHP_EOL;"
php composer-setup.php
php -r "unlink('composer-setup.php');"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think I'll fix this in a new PR to not block red builds on master

@feelepxyz feelepxyz merged commit 5b9773a into main Jul 17, 2020
@feelepxyz feelepxyz deleted the feelepxyz/fix-docker-dev-shell branch July 17, 2020 13:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants