Skip to content

WIP: Apache webserver handles TLS on Ironic and Ironic-inspector sides#562

Closed
namnx228 wants to merge 1 commit intometal3-io:masterfrom
Nordix:apache-reverse-proxy-nam
Closed

WIP: Apache webserver handles TLS on Ironic and Ironic-inspector sides#562
namnx228 wants to merge 1 commit intometal3-io:masterfrom
Nordix:apache-reverse-proxy-nam

Conversation

@namnx228
Copy link
Copy Markdown
Member

@namnx228 namnx228 commented Nov 23, 2020

This PR adds an option to let Apache web server handle TLS on Ironic and Ironic-inspector side.
Related PRs:
metal3-io/ironic-image#230
metal3-io/baremetal-operator#728
metal3-io/ironic-inspector-image#70

@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 23, 2020
@namnx228 namnx228 changed the title WIP: Apache reverse proxy nam WIP: Apache reverse proxy Nov 23, 2020
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 1d80410 to fe7117e Compare December 1, 2020 11:16
@namnx228 namnx228 changed the title WIP: Apache reverse proxy Apache webserver handles TLS on Ironic and Ironic-inspector side Dec 1, 2020
@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 Dec 1, 2020
@namnx228 namnx228 changed the title Apache webserver handles TLS on Ironic and Ironic-inspector side Apache webserver handles TLS on Ironic and Ironic-inspector sides Dec 1, 2020
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Dec 1, 2020

/assign @maelk
/cc @dtantsur
/cc @derekhiggins
/cc @fmuyassarov
/cc @furkatgofurov7
/cc @zaneb

Copy link
Copy Markdown
Member

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

/lgtm

Comment thread lib/common.sh Outdated
export IRONIC_DATA_DIR="$WORKING_DIR/ironic"
export IRONIC_IMAGE_DIR="$IRONIC_DATA_DIR/html/images"
export IRONIC_KEEPALIVED_IMAGE=${IRONIC_KEEPALIVED_IMAGE:-"quay.io/metal3-io/keepalived"}
#TODO: Update vars.md
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit:

Suggested change
#TODO: Update vars.md

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 1, 2020
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 2d35eff to 20e249b Compare December 16, 2020 16:16
@metal3-io-bot
Copy link
Copy Markdown
Collaborator

New changes are detected. LGTM label has been removed.

@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 16, 2020
@namnx228 namnx228 changed the title Apache webserver handles TLS on Ironic and Ironic-inspector sides WIP: Apache webserver handles TLS on Ironic and Ironic-inspector sides Dec 16, 2020
@metal3-io-bot metal3-io-bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 16, 2020
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch 2 times, most recently from 73ae4ea to 2d19311 Compare January 13, 2021 13:13
@metal3-io-bot
Copy link
Copy Markdown
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: namnx228
To complete the pull request process, please assign maelk
You can assign the PR to them by writing /assign @maelk in a comment when ready.

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

Details 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 removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 13, 2021
@namnx228 namnx228 changed the title WIP: Apache webserver handles TLS on Ironic and Ironic-inspector sides Apache webserver handles TLS on Ironic and Ironic-inspector sides Jan 13, 2021
@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 Jan 13, 2021
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch 2 times, most recently from 022098a to f7a0f57 Compare January 22, 2021 09:27
@metal3-io-bot metal3-io-bot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jan 22, 2021
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from 4662481 to f22b468 Compare January 28, 2021 09:36
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from f22b468 to c174f9e Compare March 9, 2021 14:34
@namnx228
Copy link
Copy Markdown
Member Author

/test-integration
/test-centos-integration

Copy link
Copy Markdown
Member

@smoshiur1237 smoshiur1237 left a comment

Choose a reason for hiding this comment

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

Lets pass the CI. Otherwise Looks good to me.

Copy link
Copy Markdown
Member

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

Question(s) inline:

Comment thread lib/common.sh Outdated
export IRONIC_IMAGE_DIR="$IRONIC_DATA_DIR/html/images"
export IRONIC_KEEPALIVED_IMAGE=${IRONIC_KEEPALIVED_IMAGE:-"quay.io/metal3-io/keepalived"}
# Option to use reverse proxy on ironic-inspector
export INSPECTOR_REVERSE_PROXY_SETUP=${INSPECTOR_REVERSE_PROXY_SETUP:-"false"}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I might be missing the bits, but are not we supposed to have this option enabled by default? AFAIK, IRONIC_TLS_SETUP is used by default.
Also, vars.md needs to be updated with new var.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was not fully clear in prev. comment, maybe. Is it the case that we will have 2 options depending on IRONIC_TLS_SETUP?:

  • IRONIC_TLS_SETUP=false ==> INSPECTOR_REVERSE_PROXY_SETUP=false;

  • IRONIC_TLS_SETUP=true ==> INSPECTOR_REVERSE_PROXY_SETUP=false/true;

Copy link
Copy Markdown
Member Author

@namnx228 namnx228 Mar 10, 2021

Choose a reason for hiding this comment

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

The reason for INSPECTOR_REVERSE_PROXY_SETUP to be false is to pass the CI. Otherwise, this PR needs the PR metal3-io/baremetal-operator#728 to go in before it can pass the CI.
Thanks for reminding me about the vars. Will do now

Copy link
Copy Markdown
Member

@fmuyassarov fmuyassarov left a comment

Choose a reason for hiding this comment

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

LGTM, squashing the commits would be good I think.

@fmuyassarov
Copy link
Copy Markdown
Member

Sorry I forgot to mention, you probably need to add the image for upgrade tests as well

@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from c174f9e to 4e24e33 Compare March 11, 2021 12:42
@namnx228
Copy link
Copy Markdown
Member Author

/hold

@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 Mar 15, 2021
@namnx228 namnx228 changed the title Apache webserver handles TLS on Ironic and Ironic-inspector sides WIP: Apache webserver handles TLS on Ironic and Ironic-inspector sides Mar 15, 2021
@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 Mar 15, 2021
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch 2 times, most recently from 8874f50 to e7de388 Compare March 22, 2021 13:22
Add option to let Apache webserver handle TLS on Ironic and Ironic-inspector

Remove the option to choose whether to use WSGI or not


Remove the option to use reverse proxy for inspector


Option to use reverse proxy on ironic inspector


Add http_reverse_proxy to the container cleaning list 


Allow `make clean` to delete httpd-reverse-proxy


Add new variable to vars.md


Use the ironic-inspector reverse proxy <=> when ironic tls is used


Revert "Add new variable to vars.md"

This reverts commit 39e10ba9079700c835d381f556247c81c6d1738c.
@namnx228 namnx228 force-pushed the apache-reverse-proxy-nam branch from e7de388 to 91131c8 Compare March 23, 2021 16:53
@metal3-io-bot metal3-io-bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Mar 23, 2021
@namnx228
Copy link
Copy Markdown
Member Author

/test-centos-integration
/test-integration

@kashifest
Copy link
Copy Markdown
Member

@namnx228 Whats the update with this PR?

@namnx228
Copy link
Copy Markdown
Member Author

I think we can close this PR. It is not needed anymore.

@namnx228 namnx228 closed this May 10, 2021
@namnx228 namnx228 deleted the apache-reverse-proxy-nam branch September 29, 2021 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants