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

Enable dev env on AAP-15607 #562

Closed
wants to merge 15 commits into from

Conversation

msmagnanijr
Copy link
Contributor

@msmagnanijr msmagnanijr requested a review from a team as a code owner December 6, 2023 21:55
@msmagnanijr msmagnanijr changed the title [WIP] - Enable dev env on AAP-15607 Enable dev env on AAP-15607 Dec 11, 2023
@msmagnanijr msmagnanijr force-pushed the AAP-15607-compose branch 2 times, most recently from 69011ac to 45d85af Compare December 11, 2023 12:35
@Alex-Izquierdo
Copy link
Contributor

Two questions:

  • Why a new different docker-compose? it is just a new volume mounts, don't?
  • Where are the certs? Why not create a wildcard selfsigned within the repo so everything can work OOB?

@msmagnanijr
Copy link
Contributor Author

Two questions:

  • Why a new different docker-compose? it is just a new volume mounts, don't?

I spoke to @mkanoor about this, and the idea was not to mix up the settings. There was something else about MacOS that I can't remember now.

  • Where are the certs? Why not create a wildcard selfsigned within the repo so everything can work OOB?

I've compiled a document with a step-by-step guide on generating valid certificates using Red Hat's internal CA, but unfortunately, it still didn't work. If, in the end, we have to resort to using self-signed certificates, I agree to have everything prepared in the repo.

@mkanoor what do you think?

@Alex-Izquierdo
Copy link
Contributor

I think @mkanoor meant to do this in a separated PR and because the changes for mac will be different than for linux. But I don't see a reason to have a new docker-compose.

Regarding the certs. I think it is not worth to add complexity and/or manual steps to deploy the environment. The goal is to work in a setup with SSL to be closer to production, not to provide a real secure environment. A self-signed cert for the development/stage environment inside the repo is a common approach and completely acceptable to me.

@mkanoor
Copy link
Contributor

mkanoor commented Dec 11, 2023

@Alex-Izquierdo We are trying to get certs from our internal CA and try this end to end with SSL for
eda-ui -> eda-api
eda-api -> postgres
worker -> postgres
ansible-rulebook -> Postgres (pg notify/listen)

While we have certs we have to make sure that we test everything works with it, but there is a manual step to get the certs from our CA which takes about 1 day. Since not all developers would be using this from the get go it would take a while for everyone to get the certs. The docker-compose-dev-ssl.yml is the file which we would eventually keep once everyone has migrated over. Since we have to mount the certs locally from every developers machine to the containers, if we have a single docker compose we would have to make the mount conditional or keep a separate file. Keeping it separate with an ssl suffix will help in development without breaking everyone else's env.

@Alex-Izquierdo
Copy link
Contributor

Ok, that makes sense.

@msmagnanijr msmagnanijr force-pushed the AAP-15607-compose branch 2 times, most recently from 40062ea to 1afb1d1 Compare January 11, 2024 14:04
@msmagnanijr
Copy link
Contributor Author

@ansible/eda-maintainers whenever you get a chance, could you please review this? tks

@ttuffin
Copy link
Contributor

ttuffin commented Mar 27, 2024

@msmagnanijr can you resolve the conflict pls?

@msmagnanijr
Copy link
Contributor Author

@msmagnanijr can you resolve the conflict pls?

Yep, I am working on it.

@@ -199,4 +199,4 @@ volumes:

networks:
service-mesh:
name: service-mesh
name: service-mesh
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you don't have rebased it properly, this change is already on main. https://github.com/ansible/eda-server/blob/main/tools/docker/docker-compose-dev.yaml#L202

Copy link
Contributor

Choose a reason for hiding this comment

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

have you look at this?

@bzwei
Copy link
Contributor

bzwei commented Apr 1, 2024

@msmagnanijr I don't have enough expertise to review this. How to use this work? Is it to do docker compose based on tools/docker/docker-compose-tls.yaml then it will automatically enable the ssl connection to DB with all key and cert provided out of box? Otherwise can you document the procedure?

@msmagnanijr
Copy link
Contributor Author

msmagnanijr commented Apr 1, 2024

@msmagnanijr I don't have enough expertise to review this. How to use this work? Is it to do docker compose based on tools/docker/docker-compose-tls.yaml then it will automatically enable the ssl connection to DB with all key and cert provided out of box? Otherwise can you document the procedure?

Yes, Mahdu asked me to create this. Let me just give one last review and I'll share the doc.

@bzwei I added the doc link here https://issues.redhat.com/browse/AAP-15607 tks

Copy link
Contributor

@Alex-Izquierdo Alex-Izquierdo left a comment

Choose a reason for hiding this comment

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

Do we need a new docker-compose file? Would make sense to merge it with the existing https://github.com/ansible/eda-server/blob/main/tools/docker/docker-compose-dev-redis-tls.yaml ?

@msmagnanijr
Copy link
Contributor Author

Do we need a new docker-compose file? Would make sense to merge it with the existing https://github.com/ansible/eda-server/blob/main/tools/docker/docker-compose-dev-redis-tls.yaml ?

I think it makes sense now. I think we had thought about creating another one for some reason so as not to impact the compose-dev

@msmagnanijr
Copy link
Contributor Author

Closing since I'm dealing with this here: #907

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

5 participants