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

Add containerization for the ipa-tuura+Apache service #47

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

antoniotorresm
Copy link
Collaborator

Add Containerfile and podman-compose files that containerize ipa-tuura using httpd as HTTPS server. This replaces the previous Dockerfile.test, using Apache HTTPS server instead of the insecure built-in Django HTTP server.

By running the podman-compose.yaml, the container is built and deployed with the needed port mapping.

Copy link
Member

@Tiboris Tiboris left a comment

Choose a reason for hiding this comment

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

Hi, @antoniotorresm In some repositories we have used this registry to not reach the pull limit from docker hub. Maybe you can consider the same.

Containerfile Outdated Show resolved Hide resolved
@f-trivino f-trivino self-requested a review June 6, 2023 13:29
@Tiboris
Copy link
Member

Tiboris commented Jun 6, 2023

And because of my fault a rebase on top of #48 would be needed to get better action results

@Tiboris Tiboris closed this Jun 6, 2023
@Tiboris Tiboris reopened this Jun 6, 2023
@Tiboris
Copy link
Member

Tiboris commented Jun 6, 2023

Weird, the automation does not pick the PR

@@ -64,7 +64,7 @@ jobs:
${{ ( github.event_name != 'pull_request' && github.ref == 'refs/heads/main' && format('{0}:latest', env.IMAGE_REGISTRY_QUAY) ) || '' }}
${{ ( github.event_name != 'pull_request' && github.ref_type == 'tag' && format('{0}:{1}', env.IMAGE_REGISTRY_QUAY, github.ref_name) ) || '' }}
containerfiles: |
./Dockerfile.test
./Containerfile
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should keep using a Development mode container for gating workflow. Maybe we should improve the naming?

@@ -0,0 +1,11 @@
version: "3"
Copy link
Collaborator

Choose a reason for hiding this comment

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

this file can be just an example on how to deploy in production mode, I think we should set ENVVARS in addition, check:

#43

[alternate_names]
# Extra domain names to associate with our cert
# - These can be a mix of wildcard, IP address, subdomain, etc.
DNS.1 = *.ipa.test
Copy link
Collaborator

Choose a reason for hiding this comment

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

we need to think how to handle these hard-coded values.

@@ -0,0 +1,71 @@
# Key:
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we could delete all comments and keep just the settigns.

Copy link
Collaborator

@f-trivino f-trivino left a comment

Choose a reason for hiding this comment

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

thanks for the PR @antoniotorresm , I added few comments. I would also improve the commit message and include the issue ticket:

Related: #29

I'm not sure if we are ready to switch everything to prod. This PR could be just the configuration files so that it is simple to deploy in prod until we figure out how to test this mode.

@antoniotorresm
Copy link
Collaborator Author

I have restored the Dockerfile.test and moved the new Containerfile to a prod directory, in order to preserve the test environment. Added needed environment variables for Django to the podman-compose file as well.

We can probably keep ipa.conf as is while we figure out how to integrate Certbot.

@Tiboris
Copy link
Member

Tiboris commented Jun 7, 2023

I think gating should use production Dockerfile whatever that means in gating.yaml definition we can later just exec the python3 src/ipa-tuura/manage.py test as a step after the container will be built. Or if we want to have both Dockerfiles or as you like Containerfiles we should share as many layers as possible from each other. And if it is only about running unit tests we can do it outside of the container even in pre-commit hook.

src/prod/Containerfile Outdated Show resolved Hide resolved
src/prod/conf/ipa.conf Outdated Show resolved Hide resolved
Copy link
Collaborator

@f-trivino f-trivino left a comment

Choose a reason for hiding this comment

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

@antoniotorresm added few comments.

Add Containerfile and podman-compose files that containerize ipa-tuura
using Apache as HTTPS server.

By running the podman-compose.yaml, the container is built and deployed
with the needed port mapping and environment variables.

Related: freeipa#29
Signed-off-by: Antonio Torres <antorres@redhat.com>
@antoniotorresm
Copy link
Collaborator Author

Thanks, pushed requested changes.

Copy link
Collaborator

@f-trivino f-trivino left a comment

Choose a reason for hiding this comment

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

LGTM.

@f-trivino f-trivino merged commit b1472aa into freeipa:main Jun 13, 2023
4 checks passed
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