-
Notifications
You must be signed in to change notification settings - Fork 68
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
feat: Integrate Tanzu Sources (vSphere/Horizon) for Knative #1075
Conversation
name: default | ||
namespace: vmware-functions | ||
address: #@ "https://"+horizon | ||
#@ if horizonTls == "True": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to double-check: if horizonTls == "True"
means disable TLS or enable TLS? Current wording is confusing in conjunction with skipTLSVerify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's correct, I most likely kept the variable name "simple" when working on vSphere sources from event-router, so just dup'ed variable name here but it is indeed reading from https://github.com/vmware-samples/vcenter-event-broker-appliance/blob/development/files/setup.sh#L34
Happy to modify for clarity purposes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, would be great to have those two align, i.e. if horizonTls == "True"
skipTLSVerify
should be false
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
files/setup-06-vsphere-sources.sh
Outdated
echo -e "\e[92mCreating vSphere Secret ..." > /dev/console | ||
# Create vSphere Secret | ||
if [ ${VCENTER_DISABLE_TLS} == "True" ]; then | ||
kn vsphere auth create \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bit weird that you deploy most of it using YAML/ytt and here use a CLI. Why not use manifests?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I started out w/vSphere Sources which provided kn plugin ... I guess looking at Horizon Sources, I had no choice but perhaps might be good to just keep it consistent. I wasn't sure if there was a preference to leverage the plugin over manifests
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, no benefit I guess in using the CLI, so let's go for consistency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
@@ -13,6 +13,11 @@ VEBA_CONFIG_FILE=/root/config/veba-config.json | |||
TINYWWW_TEMPLATE=/root/config/tinywww/templates/tinywww-template.yaml | |||
TINYWWW_CONFIG=/root/config/tinywww/tinywww.yaml | |||
|
|||
# Basic Auth for TinyWWW endpoints | |||
kubectl -n vmware-system create secret generic basic-auth \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remind me, why and where is this needed now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To access any of the endpoints served by TinyWWW - /events, /bootstrap, etc.
Basically, we don't want someone to be able to view events or debug logs w/o some sort of auth
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK. And this is not handled by an Ingress? I thought we had those endpoints exposed through ingress which also has auth?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, we've always had it like this
files/setup-05-knative.sh
Outdated
if [ ${TANZU_SOURCES_DEBUG} -eq "True" ]; then | ||
kubectl -n vmware-sources get cm config-logging -o yaml > /tmp/vmware-sources-config-logging.yaml | ||
sed -i 's/"level": "info"/"level": "debug"/g' /tmp/vmware-sources-config-logging.yaml |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this used/applied anyhwere? Not sure I'm understanding what this is supposed to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦅 👁️ No, this was temp way of enabling debugging w/sources. Thanks to Gaberiel, this is now part of YTT setup for Sources
Let me remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha, ok. 🦅 👁️ can still do it 🤣
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved
e3dc440
to
8aa1ca7
Compare
Closes: 1066 Signed-off-by: William Lam <wlam@vmware.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
This change refactors VEBA to now use Tanzu Sources for Knative when configuring either vSphere or Horizon sources and maintains event-router for Webhook processor configurations.
In addition, there were few optimizations that were needed including updating the VEBA VM hardware configuration which had some intermittent build errors preventing consistent builds.
Pull Request Checklist
🚨 Please review the guidelines for contributing to this repository.
WIP
keyword in the title of your PR if you are not ready for reviewChange Type
What types of changes does your code introduce to the VMware Event Broker Appliance?
Put an
x
in all boxes that applyPlease check the type of change your PR introduces:
Resolved Issues
Closes: #1066
Testing Verification
Additional Information
If you have any questions/comments, feel free to reach out to team on Slack #vcenter-event-broker-appliance
Thank you from the VEBA Team! 🥳