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

Update the Helm Chart to set an AIRBYTE_API_HOST in OSS and Enterprise #310

Closed
wants to merge 2 commits into from
Closed

Update the Helm Chart to set an AIRBYTE_API_HOST in OSS and Enterprise #310

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8f8b4e5
Use the webapp-url when deploying as Enterprise. This is probably bro…
bgroff Mar 18, 2024
834a328
Update the chart to do the right thing in Enterprise and OSS.
bgroff Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions charts/airbyte-server/templates/deployment.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ spec:
value: "true"
{{- end }}
{{- if eq .Values.global.deploymentMode "oss" }}
- name: AIRBYTE_API_HOST
valueFrom:
configMapKeyRef:
name: {{ .Release.Name }}-airbyte-env
key: AIRBYTE_API_HOST
- name: AIRBYTE_VERSION
valueFrom:
configMapKeyRef:
Expand Down
2 changes: 1 addition & 1 deletion charts/airbyte/templates/env-configmap.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ data:
KEYCLOAK_INTERNAL_HOST: localhost # just a placeholder so that nginx template is valid - shouldn't be used when edition isn't "pro"
{{- end }}
CONNECTOR_BUILDER_API_HOST: {{ .Release.Name }}-airbyte-connector-builder-server-svc:{{ index .Values "connector-builder-server" "service" "port" }}
AIRBYTE_API_HOST: {{ .Release.Name }}-airbyte-api-server-svc:{{ index .Values "airbyte-api-server" "service" "port" }}
AIRBYTE_API_HOST: {{ printf "%s/api/public/v1" (index $airbyteYmlDict "webapp-url") | quote }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have no idea what happens here is the airbyteYmlDict is not set. This will happen in OSS. Not sure what we should set for this value in OSS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok after looking at this more here's the rundown.

When we try to build say a pagination next URL, we take the AIRBYTE_API_HOST and grab i.e. the connections path (a constant with value /api/public/v1/connections), remove the /api/public prefix, and then smash them together to get the URL that appears in the next param of the response.

I think if we set the AIRBYTE_API_HOST to http://airbyte-server-svc.cluster.local/api/public or whatever the equivalent is, we'll still remove the /api/public off the path, but the API Host will still contain it and we'll be able to correctly direct folks. Should definitely test before we merge, but I think that will work for us.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to include the v1 at the end of the path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For Enterprise, the majority of the traffic will be coming in through an ingress, so we will want the external host name. I am good with defaulting this to the {{ Release.name }}-airbyte-server/api/public though.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense to me. To summarize:

  • In Cloud/Enterprise we'll expect an ingress to be present where we'll rewrite the path from something like api.cloud-customer.com -> airbyte-server/api/public and have an AIRBYTE_API_HOST value of api.cloud-customer.com
  • In OSS (and some Enterprise cases) we'll expect no ingress to be present which will mean all calls will be made directly to airbyte-server/api/public and the AIRBYTE_API_HOST value will be airbyte-server/api/public.

Does that seem accurate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good enough for now?

{{- if $.Values.global.jobs.kube.annotations }}
JOB_KUBE_ANNOTATIONS: {{ $.Values.global.jobs.kube.annotations | include "airbyte.flattenMap" | quote }}
{{- end }}
Expand Down
Loading