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.

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

Implement nginx proxy for lab helm chart #4926

Merged
merged 8 commits into from
Mar 22, 2021
Merged

Conversation

planetf1
Copy link
Member

  • Adds new nginx container
  • configures nginx using template
  • directs front end requests at static UI, backend at ui server chassis
  • utilizes secrets for tls cert/key and configmap for nginx template
  • environment provided by k8s deployment
  • certs currently copied in-situ into a secret yaml file (not a secure approach - initial step)

Currently testing

Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
@planetf1 planetf1 marked this pull request as draft March 19, 2021 09:00
@planetf1
Copy link
Member Author

Debugging the specific cert chosen for now:

2021/03/19 08:51:44 [emerg] 1#1: cannot load certificate "/etc/nginx/ssl/tls.crt": PEM_read_bio_X509_AUX() failed (SSL: error:0909006C:PEM routines:get_name:no start line:Expecting: TRUSTED CERTIFICATE)
nginx: [emerg] cannot load certificate "/etc/nginx/ssl/tls.crt": PEM_read_bio_X509_AUX() failed (SSL: error:0909006C:PEM routines:get_name:no start line:Expecting: TRUSTED CERTIFICATE)

The primary purpose of this initial change is to get something working to allow for zuul removal.
A later PR may be used to ensure a user specified secret may be used instead

@planetf1
Copy link
Member Author

@lpalashevski How familar are you with nginx's certificate needs? Currently I am basing the encoded certificate on those we generate under open-metadata-resources/open-metadata-deployment/certificates as a proof point, Just trying to debug the specific nature of the problem with the certs (where every tool seems different)

@planetf1
Copy link
Member Author

planetf1 commented Mar 19, 2021

To view the certs one can use

kubectl get secret lab-nginx-ssl -o json | jq '.data | map_values(@base64d)'

Both look fine, and tls.crt begins with '-----BEGIN TRUSTED CERTIFICATE-----'

@planetf1
Copy link
Member Author

planetf1 commented Mar 19, 2021

The error was caused by 2 problems in the helm template for nginx
a) I had only a tls.cert, with the contents of tls.key (mapping error)
b) I did not specify the private key password, and have added a new volume for this

Not secure, but proving the concepts.

/docker-entrypoint.sh: Configuration complete; ready for start up

Next step is to see if nginx is actually working, and tweak the config accordingly.

Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>

Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
Signed-off-by: Nigel Jones <nigel.l.jones+git@gmail.com>
@planetf1
Copy link
Member Author

planetf1 commented Mar 19, 2021

Current status - the existing UI does not work. Nginx is now being configured correctly and returns the same result.
cc: @bogdan-sava @lpalashevski @sarbull

Ie I think this pr is good, but the underlying ui is currently broken both using this mechanism or the current default using Zulu.

This needs to be investigated and fixed first

@planetf1
Copy link
Member Author

See also odpi/egeria-ui#111
If we make a change that involves the static UI switching to a different port (as it does in that PR) then I will need to adjust the ports here accordingly

Note also that once merged I still need to fix up docker compose before we can remove zuul

Finally note that in any case the polymer UI is currenly not working as per odpi/egeria-ui#110

@planetf1 planetf1 marked this pull request as ready for review March 22, 2021 13:25
@planetf1
Copy link
Member Author

This is ready to be merged.
As per #4549 still need to update compose & make some doc updates in readmes, notebook.

@planetf1 planetf1 enabled auto-merge March 22, 2021 13:31
@planetf1 planetf1 merged commit d506306 into odpi:master Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants