Skip to content

Apache webserver handles TLS on Ironic#728

Merged
metal3-io-bot merged 1 commit intometal3-io:masterfrom
Nordix:ironic-apache-reverse-proxy-nam
Mar 17, 2021
Merged

Apache webserver handles TLS on Ironic#728
metal3-io-bot merged 1 commit intometal3-io:masterfrom
Nordix:ironic-apache-reverse-proxy-nam

Conversation

@namnx228
Copy link
Copy Markdown
Member

@namnx228 namnx228 commented Nov 23, 2020

This PR let Apache web server handle TLS on Ironic
Related PRs:
metal3-io/ironic-image#230 (This PR needs to be merged before the current PR can pass the CI)
metal3-io/ironic-inspector-image#70

@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 Nov 23, 2020
@metal3-io-bot metal3-io-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 23, 2020
@dhellmann
Copy link
Copy Markdown
Member

Do we want to use a "reverse proxy" or mod_wsgi? @dtantsur what is the recommendation for deploying Ironic outside of kubernetes?

If we don't want to use mod_wsgi, is there any reason not to use a sidecar for the TLS proxy?

@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch from eb51cfe to ae9fbe5 Compare December 1, 2020 11:21
@namnx228 namnx228 changed the title WIP: Apache reverse proxy Apache webserver handles TLS on Ironic and Ironic-inspector sides 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 force-pushed the ironic-apache-reverse-proxy-nam branch from ae9fbe5 to 0cf4650 Compare December 1, 2020 11:27
@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 Dec 1, 2020
@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch 2 times, most recently from 1f7eee3 to 647b7f9 Compare December 1, 2020 11:30
@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Dec 1, 2020

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

@namnx228
Copy link
Copy Markdown
Member Author

namnx228 commented Dec 1, 2020

Do we want to use a "reverse proxy" or mod_wsgi? @dtantsur what is the recommendation for deploying Ironic outside of kubernetes?

If we don't want to use mod_wsgi, is there any reason not to use a sidecar for the TLS proxy?

@dhellmann Yes, we use mod_wsgi for Ironic-api, and use proxy for Ironic-conductor and ironic inspector. The proxy is deployed in the same container with the application (the conductor, the inspector) to improve security.

@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch from 647b7f9 to 0002778 Compare December 16, 2020 16:34
@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 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 the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 16, 2020
@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch 2 times, most recently from a3378c0 to b866e68 Compare January 11, 2021 10:21
@namnx228 namnx228 changed the title WIP: Apache webserver handles TLS on Ironic and Ironic-inspector sides Apache webserver handles TLS on Ironic Jan 11, 2021
- configMapRef:
name: ironic-bmo-configmap
- name: httpd-reverse-proxy
image: quay.io/metal3-io/ironic-inspector
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.

Should it be inspector image or ironic image?

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.

@kashifest
Copy link
Copy Markdown
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 15, 2021
Comment on lines -45 to -58
- name: ironic-httpd
image: quay.io/metal3-io/ironic
imagePullPolicy: Always
securityContext:
capabilities:
add: ["NET_ADMIN"]
command:
- /bin/runhttpd
volumeMounts:
- mountPath: /shared
name: ironic-data-volume
envFrom:
- configMapRef:
name: ironic-bmo-configmap
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.

you should not remove this. This is the webserver for IPA image and other PXE related files

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@maelk I remove it because the httpd server which works as a WSGI server inside the ironic-api pod will also serve the IPA image.

@maelk maelk self-requested a review March 16, 2021 15:10
@maelk
Copy link
Copy Markdown
Member

maelk commented Mar 16, 2021

/lgtm

@namnx228
Copy link
Copy Markdown
Member Author

/assign @dtantsur

@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch from 6758178 to 04f0acb Compare March 17, 2021 07:39
@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021
@namnx228
Copy link
Copy Markdown
Member Author

/test-integration
/test-centos-integration

Comment thread tools/deploy.sh Outdated
Comment thread tools/run_local_ironic.sh
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.

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021
@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch from 04f0acb to 6cfc9b4 Compare March 17, 2021 12:19
@metal3-io-bot metal3-io-bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021
Add option to let Apache web server handle TLS on Ironic and Ironic-inspector sides

Force the use of WSGI and remove the httpd container


Remove the option to use reverse proxy for inspector


Add option to use reverse proxy on ironic inspector
@namnx228 namnx228 force-pushed the ironic-apache-reverse-proxy-nam branch from 6cfc9b4 to cbdff6d Compare March 17, 2021 12:20
@namnx228
Copy link
Copy Markdown
Member Author

/test-integration
/test-centos-integration

@metal3-io-bot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: elfosardo, maelk, namnx228

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

The pull request process is described 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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 17, 2021
@kashifest
Copy link
Copy Markdown
Member

/lgtm

@metal3-io-bot metal3-io-bot added the lgtm Indicates that a PR is ready to be merged. label Mar 17, 2021
@metal3-io-bot metal3-io-bot merged commit 82fd2d7 into metal3-io:master Mar 17, 2021
@namnx228
Copy link
Copy Markdown
Member Author

Thanks very much everyone for your review and approval.

elfosardo added a commit to elfosardo/ironic-image that referenced this pull request Mar 26, 2021
Now that metal3-io/baremetal-operator#728 has
merged, we can remove net-tools installation command.
elfosardo added a commit to elfosardo/ironic-image that referenced this pull request Mar 29, 2021
Now that metal3-io/baremetal-operator#728 has
merged, we can remove net-tools installation command and the netstat
entry in runironic-api.
elfosardo added a commit to elfosardo/ironic-image that referenced this pull request Mar 29, 2021
Now that metal3-io/baremetal-operator#728 has
merged, we can remove net-tools installation command and the netstat
entry in runironic-api.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. 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

Development

Successfully merging this pull request may close these issues.

8 participants