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.

Sign up for GitHub

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

Jump to bottom

🌱 Add tls certs to fakeIPA config #1464

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🌱 Add tls certs to fakeIPA config #1464

wants to merge 1 commit into from

Conversation

mboukhalfa
Copy link
Member

@mboukhalfa mboukhalfa commented Oct 28, 2024 •
edited
Loading

After merging metal3-io/utility-images#26 we can set TLS certs path in fakeIPA config to enable TLS

This needs to wait for sushy-tools image and release with the following patch :https://review.opendev.org/c/openstack/sushy-tools/+/933551

/hold

@metal3-io-bot
Copy link
Collaborator

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@metal3-io-bot metal3-io-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 28, 2024
@metal3-io-bot metal3-io-bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 28, 2024
@mboukhalfa mboukhalfa force-pushed the Add-tls-config-to-fakeIPA/mboukhalfa branch 2 times, most recently from a396498 to b075659 Compare October 28, 2024 17:20
@mboukhalfa mboukhalfa marked this pull request as ready for review October 28, 2024 17:22
@metal3-io-bot metal3-io-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 28, 2024
Rozzii
Rozzii requested changes Oct 29, 2024
View reviewed changes
Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

Generally looks okay, with a comment.

vm-setup/roles/virtbmc/tasks/setup_tasks.yml
@@ -149,6 +149,7 @@
SUSHY_EMULATOR_VMEDIA_VERIFY_SSL = {{ sushy_vmedia_verify_ssl }}
SUSHY_EMULATOR_AUTH_FILE = "/root/sushy/htpasswd"
SUSHY_EMULATOR_FAKE_DRIVER = True
EXTERNAL_NOTIFICATION_URL = "https://localhost:9999"
Copy link
Member

Choose a reason for hiding this comment

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

Could you please give this a a name that more explicitly ties the variable to FAKE-IPA ?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's in the sushy-tools and I wanted to be explicit using EXTERNAL_NOTIFICATION prefix it can be fake-ipa or any external component

@mboukhalfa
Copy link
Member Author

/hold
Waiting for sushy-tools image with release v1.3.1

@metal3-io-bot metal3-io-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 29, 2024
@Rozzii Rozzii self-requested a review October 30, 2024 07:31
Rozzii
Rozzii reviewed Oct 30, 2024
View reviewed changes
Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Oct 30, 2024
Rozzii
Rozzii previously approved these changes Oct 30, 2024
View reviewed changes
@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 30, 2024
lentzi90
lentzi90 reviewed Nov 1, 2024
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. label Dec 12, 2024
@tuminoid
Copy link
Member

Hmm, metal3-io/utility-images#26 was merged long time ago, and this has been sitting with approve+lgtm for long time, until now it rotted.

Who is going to finalize this? @Rozzii , @mquhuy ?

@Rozzii
Copy link
Member

Rozzii commented Dec 12, 2024

Hmm, metal3-io/utility-images#26 was merged long time ago, and this has been sitting with approve+lgtm for long time, until now it rotted.

Who is going to finalize this? @Rozzii , @mquhuy ?

I don't see me doing this in the near future or even in the medium.

@mquhuy
Copy link
Member

mquhuy commented Dec 12, 2024

Hmm, metal3-io/utility-images#26 was merged long time ago, and this has been sitting with approve+lgtm for long time, until now it rotted.

Who is going to finalize this? @Rozzii , @mquhuy ?

I've honestly forgotten what this was 😅, but I can put it in my TODO list.

@mboukhalfa @mquhuy
Add tls certs to fakeIPA config
fd00fb6 
Signed-off-by: Mohammed Boukhalfa <[email protected]>
@mquhuy mquhuy force-pushed the Add-tls-config-to-fakeIPA/mboukhalfa branch from b075659 to fd00fb6 Compare December 12, 2024 11:17
@metal3-io-bot metal3-io-bot removed lgtm Indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. needs-rebase Indicates that a PR cannot be merged because it has merge conflicts with HEAD. labels Dec 12, 2024
@metal3-io-bot metal3-io-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Dec 12, 2024
@mquhuy
Copy link
Member

mquhuy commented Dec 13, 2024

/test metal3-dev-env-integration-test-ubuntu-main
/test metal3-centos-e2e-integration-test-release-1-8

@mquhuy
Copy link
Member

mquhuy commented Dec 16, 2024

/test metal3-centos-e2e-integration-test-release-1-8

@mquhuy
Copy link
Member

mquhuy commented Dec 17, 2024

/retest

1 similar comment
@mquhuy
Copy link
Member

mquhuy commented Dec 19, 2024

/retest

@Rozzii
Copy link
Member

Rozzii commented Dec 23, 2024

/test metal3-centos-e2e-integration-test-release-1-9

Rozzii
Rozzii reviewed Dec 23, 2024
View reviewed changes
Copy link
Member

@Rozzii Rozzii left a comment

Choose a reason for hiding this comment

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

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 23, 2024
lentzi90
lentzi90 reviewed Jan 9, 2025
View reviewed changes
Copy link
Member

@lentzi90 lentzi90 left a comment

Choose a reason for hiding this comment

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

Looks like this is ready to merge?
The indentation is wrong but I don't want it to rot again
/approve

@metal3-io-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: lentzi90

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@metal3-io-bot metal3-io-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 9, 2025
@tuminoid
Copy link
Member

tuminoid commented Jan 9, 2025

/hold Waiting for sushy-tools image with release v1.3.1

I think this is the last hold comment here, currently blocking merging. Is sushy-tools ok?

@lentzi90
Copy link
Member

lentzi90 commented Jan 9, 2025

Hmm I guess we need to figure out how to run a proper test with fake-IPA on this PR. That is what the sushy-tools image would be needed for

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rozzii Rozzii Rozzii left review comments

@lentzi90 lentzi90 lentzi90 left review comments

@elfosardo elfosardo Awaiting requested review from elfosardo

@Sunnatillo Sunnatillo Awaiting requested review from Sunnatillo

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mboukhalfa @metal3-io-bot @tuminoid @Rozzii @mquhuy @lentzi90