-
Notifications
You must be signed in to change notification settings - Fork 95
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
[WIP]THREESCALE-11020 Redis TLS certs and keys for porta and backend #1025
Conversation
6300d72
to
bbd4687
Compare
2f61cac
to
0330ce7
Compare
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.
@valerymo Overall the changes look good but need to remove generic watched-by Data-Only implementation (Predicates) as we'll implement those changes in THREESCALE-11395
Watches( | ||
&v1.Secret{}, | ||
handler.EnqueueRequestsFromMapFunc(secretToApimanagerEventMapper.Map), | ||
builder.WithPredicates(watchedByDataOnlySecretLabelPredicate), | ||
). |
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.
@valerymo I think it would be best if we remove the partial watched-by improvements from this PR and do it entirely in THREESCALE-11395.
We also want to use the same implementation (tracking changes to secret's .data
using the master hashed secret) from the apicast-operator PR.
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.
@carlkyrillos Maybe we can use combined approach ? Seems to me, there are following Pros/Cons of both methods:
- ApiCast approach - Pros: Flexable, allows implement complex logics without being constrained by predicate. Cons: Performance, checking Data in each reconciliation (if I'm not wrong)
- Predicate in Watch - Pros: Efficiency , Cons: Not Flexable if need to react to other events or changes in the future
What do you think? Thank you for your comments and review
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.
Hi @carlkyrillos I will remove my Predicate based watching from the PR from here to make Jiras purpose clear. I will send update when done.
Also -
I added suggestion to THREESCALE-11395 to consider combined
approach - use both methods.
Thank you for comments!
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.
Done. Thank you
1086bc0
to
10086fd
Compare
Hi @carlkyrillos , |
d2c23c1
to
f501e2e
Compare
f4bc009
to
516f117
Compare
@valerymo wondering about this implementation, I was looking for where you set the mount path for the cert as the cert env var should be pointing at a path to the cert and not the contents of the cert ? redis-cli -h redis-server.example.com -p 6380 --tls --cert /path/to/client-cert.pem --key /path/to/client-key.pem --cacert /path/to/ca-cert.pem
redis-cli -h redis-server.example.com -p 6380 --tls --cacert /path/to/ca-cert.pem I have confirmed this with the system team in this thread https://gitlab.cee.redhat.com/red-hat-3scale-documentation/3scale-documentation/-/merge_requests/1554#note_13233849 |
66b0c05
to
ba6ec55
Compare
4c11a56
to
ff24cf1
Compare
ff24cf1
to
bdb3201
Compare
bdb3201
to
9b1fd41
Compare
This PR is moved to new one - #1035 |
Jira: https://issues.redhat.com/browse/THREESCALE-11020
Add a way for the user to provide Redis TLS certs and keys for porta and backend
This PR is moved to new one - #1035
New PR is created from current. So all review comments that addressed here are included in new PR.
The reason of new PR - the cleanup is required - remove Certificates Paths and SSL flags that were added in current PR - from Secret; to avoid editing it by User. The cleanup is significant, so decided to close current PR (save it meanwhile) and continue with cleanup/updates on new PR.
New environment variables have been added to the existing secrets - system-redis and backend-redis
Redis TLS Enabling optional flag (boolean)
redisTLSEnabled
has been added to Apimanager CR.redisTLSEnabled
is true (deployments: system-app, system-sidekiq, backend-worker, backend-listener)REDIS_SSL, CONFIG_REDIS_SSL, CONFIG_QUEUES_SSL env will set to "true" if any other env var was set, otherwise they will set to false.
Validation-1 - check new env vars and RedisTLSEnabled boolean parameter
Example:
oc get secret backend-redis -oyaml |grep CONF |grep SSL
CONFIG_QUEUES_SSL: dHJ1ZQ==
CONFIG_REDIS_SSL: dHJ1ZQ==
~/work/56_THREESCALE-11020_3scale_RedisTLS_Secrets
Check Mutation when RedisTLSEnabled boolean parameter changed in ApiManager CR
1. redisTLSEnabled not defined in ApiManager CR
2. Add redisTLSEnabled true to ApiManager CR
3. Set redisTLSEnabled false to ApiManager CR
Validation -2, TLS Enabled, Internal Redis, Certificates mounts
Check Certificates mounts
~/go/3scale-operator oc rsh backend-worker-675b4944b8-plb6m
Defaulted container "backend-worker" out of: backend-worker, backend-redis-svc (init)
sh-4.4$ ls -l /tls
total 0
lrwxrwxrwx. 1 root 1001050000 27 Nov 12 05:48 backend-redis-ca.crt -> ..data/backend-redis-ca.crt
lrwxrwxrwx. 1 root 1001050000 31 Nov 12 05:48 backend-redis-client.crt -> ..data/backend-redis-client.crt
lrwxrwxrwx. 1 root 1001050000 32 Nov 12 05:48 backend-redis-private.key -> ..data/backend-redis-private.key
lrwxrwxrwx. 1 root 1001050000 27 Nov 12 05:48 config-queues-ca.crt -> ..data/config-queues-ca.crt
lrwxrwxrwx. 1 root 1001050000 31 Nov 12 05:48 config-queues-client.crt -> ..data/config-queues-client.crt
lrwxrwxrwx. 1 root 1001050000 32 Nov 12 05:48 config-queues-private.key -> ..data/config-queues-private.key
sh-4.4$
sh-4.4$
sh-4.4$ cat /tls/backend-redis-private.key
-----BEGIN PRIVATE KEY-----
MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCPzf6mzBvMZiEz
.......==
-----END PRIVATE KEY-----
sh-4.4$ cat /tls/backend-redis-client.crt
-----BEGIN CERTIFICATE-----
MIIDHTCCAgWgAwIBAgIUVfsq5Zmly84lBBXgJrNSSiPISoMwDQYJKoZIhvcNAQEL
......
-----END CERTIFICATE-----
sh-4.4$ cat /tls/backend-redis-ca.crt
-----BEGIN CERTIFICATE-----
MIIDQTCCAimgAwIBAgITBmyfz5m/jAo54vB4ikPmljZbyjANBgkqhkiG9w0BAQsF
.....
-----END CERTIFICATE-----
sh-4.4$ ^C
sh-4.4$ exit
command terminated with exit code 130
~/go/3scale-operator
~/go/3scale-operator
~/go/3scale-operator oc rsh backend-listener-76b477f78c-pndbf
sh-4.4$ ls -l /tls
total 0
lrwxrwxrwx. 1 root 1001050000 27 Nov 12 05:18 backend-redis-ca.crt -> ..data/backend-redis-ca.crt
lrwxrwxrwx. 1 root 1001050000 31 Nov 12 05:18 backend-redis-client.crt -> ..data/backend-redis-client.crt
lrwxrwxrwx. 1 root 1001050000 32 Nov 12 05:18 backend-redis-private.key -> ..data/backend-redis-private.key
lrwxrwxrwx. 1 root 1001050000 27 Nov 12 05:18 config-queues-ca.crt -> ..data/config-queues-ca.crt
lrwxrwxrwx. 1 root 1001050000 31 Nov 12 05:18 config-queues-client.crt -> ..data/config-queues-client.crt
lrwxrwxrwx. 1 root 1001050000 32 Nov 12 05:18 config-queues-private.key -> ..data/config-queues-private.key
sh-4.4$
sh-4.4$
sh-4.4$ exit
exit
~/go/3scale-operator oc rsh system-app-85cb4d5c64-x7fh4
Defaulted container "system-master" out of: system-master, system-provider, system-developer
sh-4.4$ ls /tls
backend-redis system-redis
sh-4.4$ ls /tls/backend-redis/
backend-redis-ca.crt backend-redis-client.crt backend-redis-private.key
sh-4.4$ ls /tls/system-redis/
system-redis-ca.crt system-redis-client.crt system-redis-private.key
sh-4.4$ cat /tls/backend-redis/backend-redis-private.key
-----BEGIN PRIVATE KEY-----
MIIEvAIBADANBgkqhkiG9w0BAQEFAASCBKYwggSiAgEAAoIBAQCPzf6mzBvMZiEz
.....
-----END PRIVATE KEY-----
sh-4.4$ cat /tls/system-redis/system-redis-private.key
replacemesh-4.4$ exit
exit
~/go/3scale-operator
~/go/3scale-operator oc rsh system-sidekiq-7555f7494b-zq6hk
Defaulted container "system-sidekiq" out of: system-sidekiq, check-svc (init)
sh-4.4$ ls /tls
backend-redis system-redis
sh-4.4$ ls /tls/backend-redis/
backend-redis-ca.crt backend-redis-client.crt backend-redis-private.key
sh-4.4$ ls /tls/system-redis/
system-redis-ca.crt system-redis-client.crt system-redis-private.key
sh-4.4$