-
Notifications
You must be signed in to change notification settings - Fork 69
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
Release 0.2.0 #55
base: master
Are you sure you want to change the base?
Release 0.2.0 #55
Conversation
* Upgrading to Terraform Provider V4
Replaces Lambda@Edge with CloudFront Function (#62) Co-authored-by: Kurt Gardiner
This also resolves #50 One question, are you able now to run the workflows against this PR? Curious to see if my PRs broke any of the inspections. |
Not on the fork. I tried upgrading the tests but it still seems to have the same issue with refspec. I'd need more time to dig into it. Once your PRs get merged to 0.2.0 the tests are run again, and the pre-commit config will make an auto-commit (as you can see here) to fix things like regeneration of terraform-docs. The only other failing test is yamllint complaining about the linelength of the buildspec of codebuild. I've actually got a comment which should ignore that specific test, but it doesn't seem to be honouring it for some reason. If you install pre-commit yourself, the tests will also run locally before it allows you to complete a commit. Failing tests will auto-fix in some cases, and then you can commit again. @z0c I'll solve the problem by adding you as a contributor, then you can branch directly on this repo. |
* Passes cloudfront_function_301_redirects to cloudfront module * Fixes missing uri param * Passes plugin version from top level module * Updates WP2Static url as repo was migrated * Adds some help of permanent redirects * Removed bad paste
Thanks @petewilcock . Actions can be a pain with forks, sometimes a workaround for this kind of issues is to run on the Next change i will branch in this repo 👍 |
|
||
cloudfront_class = var.cloudfront_class | ||
waf_acl_arn = var.waf_enabled ? module.waf[0].waf_acl_arn : null | ||
cloudfront_function_301_redirects = var.cloudfront_function_301_redirects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cloudfront_function_301_redirects = var.cloudfront_function_301_redirects | |
cloudfront_function_301_redirects = var.cloudfront_function_301_redirects | |
site_prefix = var.site_prefix |
See #66
ARG wp2static_version | ||
ARG wp2static_s3_addon_version |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each time you use a FROM
, it creates a new docker layer and these args are lost.
So, you need to pull these two ARG
declarations down below the FROM
for it to work properly. See https://benkyriakou.com/posts/docker-args-empty for more info.
Otherwise, the arguments will be empty when you try to use them below.
RUN curl https://github.com/WP2Static/wp2static/archive/refs/tags/${wp2static_version}.zip -o /tmp/serverless-wordpress-wp2static.zip | ||
RUN curl https://github.com/leonstafford/wp2static-addon-s3/archive/refs/tags/${wp2static_s3_addon_version}.zip -o /tmp/serverless-wordpress-s3-addon.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RUN curl https://github.com/WP2Static/wp2static/archive/refs/tags/${wp2static_version}.zip -o /tmp/serverless-wordpress-wp2static.zip | |
RUN curl https://github.com/leonstafford/wp2static-addon-s3/archive/refs/tags/${wp2static_s3_addon_version}.zip -o /tmp/serverless-wordpress-s3-addon.zip | |
RUN curl -Lf https://github.com/WP2Static/wp2static/archive/refs/tags/${wp2static_version}.zip -o /tmp/serverless-wordpress-wp2static.zip | |
RUN curl -Lf https://github.com/leonstafford/wp2static-addon-s3/archive/refs/tags/${wp2static_s3_addon_version}.zip -o /tmp/serverless-wordpress-s3-addon.zip |
In addition to the comment about the ARG
s needing to move down (above), these commands were silently failing.
/tmp curl https://github.com/WP2Static/wp2static/archive/refs/tags/7.1.7.zip -o /tmp/serverless-wordpress-wp2static.zip
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
/tmp ls -alh /tmp/serverless-wordpress-wp2static.zip
-rw-r--r-- 1 blimmer wheel 0 May 14 20:59 /tmp/serverless-wordpress-wp2static.zip
You need to pass -L
to follow redirects for this to work:
/tmp curl -L https://github.com/WP2Static/wp2static/archive/refs/tags/7.1.7.zip -o /tmp/serverless-wordpress-wp2static.zip
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
100 153k 0 153k 0 0 206k 0 --:--:-- --:--:-- --:--:-- 0
/tmp ls -alh /tmp/serverless-wordpress-wp2static.zip
-rw-r--r-- 1 blimmer wheel 154K May 14 21:00 /tmp/serverless-wordpress-wp2static.zip
Also, you should pass -f
so curl
will fail if it encounters a non-20x HTTP response code:
/tmp curl -Lf https://github.com/WP2Static/wp2static/archive/refs/tags/not-a-real-release.zip -o /tmp/serverless-wordpress-wp2static.zip
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0
curl: (22) The requested URL returned error: 404
✘ /tmp echo $?
22
COPY docker-entrypoint.sh /usr/local/bin/ | ||
RUN apt-get update && apt-get install -y sudo jq awscli mariadb-client && chmod +x /usr/local/bin/docker-entrypoint.sh && chmod +x /tmp/wp-cli.phar && mv /tmp/wp-cli.phar /usr/local/bin/wp \ | ||
&& rm -rf /var/lib/apt/lists/* | ||
|
||
RUN curl https://github.com/WP2Static/wp2static/archive/refs/tags/${wp2static_version}.zip -o /tmp/serverless-wordpress-wp2static.zip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm seeing some issues with using this source code directly:
According to the docs they referenced in that message, it sounds like we need to do some additional work with this source code to make it work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright, I got wp2static working with a mutli-stage build:
ARG aws_account_id
ARG aws_region
ARG ecr_repo_name
FROM php:7.4 as compile
ARG wp2static_version
WORKDIR /deps
RUN apt-get update && \
apt-get install --no-install-recommends -y wget zip unzip && \
rm -fr /var/lib/apt/lists/*
RUN wget https://raw.githubusercontent.com/composer/getcomposer.org/main/web/installer -O - -q | php -- --quiet && \
mv /deps/composer.phar /usr/local/bin/composer
RUN curl -Lf https://github.com/WP2Static/wp2static/archive/refs/tags/${wp2static_version}.zip -o /deps/serverless-wordpress-wp2static.zip && \
unzip /deps/serverless-wordpress-wp2static.zip && \
cd /deps/wp2static-${wp2static_version} && \
composer install && \
composer build serverless-wordpress-wp2static && \
mv $HOME/Downloads/serverless-wordpress-wp2static.zip /deps/serverless-wordpress-wp2static.zip
ARG aws_account_id
ARG aws_region
ARG ecr_repo_name
FROM ${aws_account_id}.dkr.ecr.${aws_region}.amazonaws.com/${ecr_repo_name}:base as wordpress
ARG wp2static_s3_addon_version
COPY ["wp-cli.phar", "/tmp/"]
COPY docker-entrypoint.sh /usr/local/bin/
RUN apt-get update && apt-get install -y sudo jq awscli mariadb-client && chmod +x /usr/local/bin/docker-entrypoint.sh && chmod +x /tmp/wp-cli.phar && mv /tmp/wp-cli.phar /usr/local/bin/wp \
&& rm -rf /var/lib/apt/lists/*
COPY --from=compile /deps/serverless-wordpress-wp2static.zip /tmp/serverless-wordpress-wp2static.zip
RUN curl -Lf https://github.com/leonstafford/wp2static-addon-s3/archive/refs/tags/${wp2static_s3_addon_version}.zip -o /tmp/serverless-wordpress-s3-addon.zip
RUN mv "$PHP_INI_DIR/php.ini-production" "$PHP_INI_DIR/php.ini"
COPY ["php.ini", "$PHP_INI_DIR/conf.d/"]
Now that plugin is passing, but the other plugin isn't working still. I'm going to call it a night and I'll see if I can get the other one working. It feels like I'm close.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
many thanks @blimmer - some thoughtful and useful comments here. I now remember another reason why I'd opted to bundle the pre-compiled of WP2Static - because the releases are source code only! I think the same is true for the S3 add-on.
I think we'll also need to be careful to remove any source files from the layer to avoid bloating out the image.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this @blimmer! These are very good points, but im quite baffled as to why my dev instance is working fine... Clearly missed something, need a closer inspection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we'll also need to be careful to remove any source files from the layer to avoid bloating out the image.
@petewilcock - if you mean you want to make sure all the composer dependencies are removed from the final build, this is exactly why I'm using a multi-stage build here 😄 TLDR; the first "FROM" is a layer has all the dev dependencies and we copy the single built zip file over to the final layer. This helps keep the final docker image small
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @blimmer I wasn't paying enough attention to the diff, looks good :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just got this working in my account with this dockerfile:
ARG aws_account_id
ARG aws_region
ARG ecr_repo_name
# This stage is used to install plugin dependencies using composer. We use a docker multi-stage build to ensure all
# these extra dependencies do not end up in the final layer.
FROM php:7.4 as compile
ARG wp2static_version
ARG wp2static_s3_addon_version
WORKDIR /deps
# Install system dependencies
RUN apt-get update && \
apt-get install --no-install-recommends -y wget zip unzip && \
rm -fr /var/lib/apt/lists/*
# Install composer
RUN wget https://raw.githubusercontent.com/composer/getcomposer.org/main/web/installer -O - -q | php -- --quiet && \
mv /deps/composer.phar /usr/local/bin/composer
# Download wp2static source code, install composer dependencies and build production plugin bundle
RUN curl -Lf https://github.com/WP2Static/wp2static/archive/refs/tags/${wp2static_version}.zip -o /deps/serverless-wordpress-wp2static.zip && \
unzip /deps/serverless-wordpress-wp2static.zip && \
cd /deps/wp2static-${wp2static_version} && \
composer install && \
composer build serverless-wordpress-wp2static && \
mv $HOME/Downloads/serverless-wordpress-wp2static.zip /deps/serverless-wordpress-wp2static.zip
# Download wp2static-s3-addon source code, install composer dependencies and build production plugin bundle
RUN curl -Lf https://github.com/leonstafford/wp2static-addon-s3/archive/refs/tags/${wp2static_s3_addon_version}.zip -o /deps/serverless-wordpress-s3-addon.zip && \
unzip /deps/serverless-wordpress-s3-addon.zip && \
cd /deps/wp2static-addon-s3-${wp2static_s3_addon_version} && \
composer install && \
composer build serverless-wordpress-s3-addon && \
mv $HOME/Downloads/serverless-wordpress-s3-addon.zip /deps/serverless-wordpress-s3-addon.zip
# Start over with a fresh wordpress layer
ARG aws_account_id
ARG aws_region
ARG ecr_repo_name
FROM ${aws_account_id}.dkr.ecr.${aws_region}.amazonaws.com/${ecr_repo_name}:base as wordpress
COPY ["wp-cli.phar", "/tmp/"]
COPY docker-entrypoint.sh /usr/local/bin/
RUN apt-get update && apt-get install -y sudo jq awscli mariadb-client && chmod +x /usr/local/bin/docker-entrypoint.sh && chmod +x /tmp/wp-cli.phar && mv /tmp/wp-cli.phar /usr/local/bin/wp \
&& rm -rf /var/lib/apt/lists/*
# Copy over built plugins for the compile layer
COPY --from=compile /deps/serverless-wordpress-wp2static.zip /tmp/serverless-wordpress-wp2static.zip
COPY --from=compile /deps/serverless-wordpress-s3-addon.zip /tmp/serverless-wordpress-s3-addon.zip
RUN mv "$PHP_INI_DIR/php.ini-production" "$PHP_INI_DIR/php.ini"
COPY ["php.ini", "$PHP_INI_DIR/conf.d/"]
The only outstanding thing you might want to think about is the PHP layer version. You might want to make that configurable in case someone is using a different version of PHP. I'm not familiar at all with the PHP ecosystem, so not sure how important matching versions is.
@@ -34,6 +35,13 @@ | |||
"readOnly": false | |||
} | |||
], | |||
"healthCheck": { | |||
"retries": 10, | |||
"command": [ "CMD-SHELL", "curl -f http://localhost:80/ || exit 1" ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@petewilcock something i came across using this branch;
If WP goes into maintenance mode (like when upgrading some plugins) the healthcheck will fai terminating the container.
This can leave the file system and/or db in a bad state. I did #65 while looking into this, had to exec to the container to manually get my dev instance back to a working state.
Going forward, the PR above also adds a var to toggle the healthcheck, which at least give a process to do maintenance by disabling it. Not great user experience though.
But think the healthcheck needs a bit more investigation, i've seen online some recommending /wp-admin/install.php
or /wp-admin/images/wordpress-logo.svg
as better healthcheck, but haven't had a chance to try because had no maintenance updates so far.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I had no idea the check could potentially fail in maintenance mode. Still not sure why it would. Even in maintenance mode, does the container not respond on port 80? Or is it getting a 301 or something that's throwing it off? Interesting...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It returns a 503
, with an actual body of this site is under maintenance, wait a bit...
kind of message.
This is the post were I saw the recommendation to /wp-admin/install.php
as a workaround bitnami/charts#3019
I haven't been able to test it myself yet.
Resolves #54
Resolves #48
Resolves #24
Resolves #57
Resolves #50