-
Notifications
You must be signed in to change notification settings - Fork 9
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
New changes #47
New changes #47
Conversation
Signed-off-by: Keyvan <keyvan@thehyve.nl>
fixes configuration issues on gateway
Add cc schema registry proxy
- Fixed health check - Separated ManagementPortal helmfile
Misc changes for ManagementPortal database checks
Add s3 proxy
add more-promasys-sync oauth config to management-portal
Add appconfig and merge radar-kubernetes
Signed-off-by: Keyvan <keyvan@thehyve.nl>
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.
Fine in general, however some variables should be generalised to merge this into RADAR-Kubernetes
@@ -22,5 +22,6 @@ data: | |||
radar_rest_sources_auth_backend;res_ManagementPortal;{{ .Values.client_secrets.radar_rest_sources_backend }};PROJECT.READ,SOURCETYPE.READ,SUBJECT.READ,SOURCE.READ;client_credentials;;;43200;86400;{}; | |||
radar_rest_sources_authorizer;res_restAuthorizer;;SOURCETYPE.READ,PROJECT.READ,SUBJECT.READ;authorization_code;https://{{ .Values.server_name }}/rest-sources/authorizer/login;;43200;86400;{}; | |||
radar_fitbit_connector;res_restAuthorizer;{{ .Values.client_secrets.radarFitbitConnector }};SUBJECT.READ;client_credentials;;;43200;;{}; | |||
radar_appconfig;res_ManagementPortal;test;SOURCETYPE.READ,MEASUREMENT.CREATE,PROJECT.READ,SUBJECT.READ,OAUTHCLIENTS.READ;{{ .Values.client_secrets.radarAppconfig }};;3600;78000;; | |||
radar_appconfig_frontend;res_appconfig;;SOURCETYPE.READ,MEASUREMENT.CREATE,PROJECT.READ,PROJECT.CREATE,PROJECT.UPDATE,SUBJECT.READ,SUBJECT.UPDATE,OAUTHCLIENTS.READ;authorization_code,refresh_token;https://{{ .Values.server_name }}/appconfig/login;3600;78000;; | |||
more_promasys_sync_client;res_ManagementPortal;{{ .Values.client_secrets.morePromasysSync }};PROJECT.READ,PROJECT.CREATE,SUBJECT.READ,SUBJECT.CREATE;client_credentials;;;43200;;{}; |
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.
Not part of radar-kubernetes
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.
Fixed in 855e582
heritage: {{ .Release.Service | quote }} | ||
type: Opaque | ||
data: | ||
root.crt: {{ .Files.Get "files/BaltimoreCyberTrustRoot.crt.pem" | b64enc | quote }} |
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.
For more general application, change this to files/root.crt
.
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.
Fixed in fbda1ed
@@ -47,14 +47,20 @@ spec: | |||
- name: SPRING_PROFILES_ACTIVE | |||
value: "prod,swagger" | |||
- name: SPRING_DATASOURCE_URL | |||
value: "jdbc:postgresql://{{ .Values.postgres.host }}:5432/managementportal" | |||
value: "jdbc:postgresql://{{ .Values.postgres.host }}:5432/managementportal?ssl=true&sslmode=verify-ca" |
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.
SSL configuration is not true in general. Perhaps instead of a postgres host, this should take a JDBC URL, e.g. {{ .Values.postgres.url }}
.
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.
Fixed in fbda1ed
base.yaml
Outdated
radarAppconfig: secret | ||
sendGrid: |
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.
We don't need a reference to sendGrid, instead smtp can be used
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.
Wouldn't it be good to reference the configurable options in here?
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.
Yes that’s fine, but they’re just plain old SMTP settings, just rename sendGrid to smtp.
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.
Fixed in 71673c7
base.yaml
Outdated
@@ -131,7 +146,15 @@ management_portal: | |||
radar_upload_frontend: secret | |||
radar_rest_sources_backend: secret | |||
radarFitbitConnector: secret | |||
morePromasysSync: secret |
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.
Not needed in RADAR-kubernetes
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.
Fixed in 855e582
helmfile.d/10-base.yaml
Outdated
- ../../etc/production.yaml | ||
secrets: | ||
- ../../secrets/production.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.
To streamline this, could we create a environment.yaml
which contains references to these production / secrets files? This would not be committed to Git, although we can add a template. Or we need to add documentation to force all users to use these locations.
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.
Waiting to find a way to solve or workaround roboll/helmfile#1454
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.
Fixed in a3568f3
Signed-off-by: Keyvan <keyvan@thehyve.nl>
Signed-off-by: Keyvan <keyvan@thehyve.nl>
Signed-off-by: Keyvan <keyvan@thehyve.nl>
* dev: Use helmfile layering for more flexibility Minor fixes and improvments Added Confluent helm charts as submodule
Merge back recent updates and additions to the charts