[bitnami/etcd] fix: healthcheck will failed when startup etcd with on… #70597
Conversation
javsalgar
added
verify
|
@andresbono @javsalgar so what's the progress of this PR? |
There was a problem hiding this comment.
Thank you for submitting this PR!
The changes you are proposing will affect several functions. Isn't adding --insecure-transport=false and --insecure-skip-tls-verify=true unconditionally too aggressive?
Maybe it could only be necessary when there is no ETCD_TRUSTED_CA_FILE? Or maybe it should be scoped to the healthcheck.sh script only.
Let me know your thoughts!
ya, could only be necessary when there is no ETCD_TRUSTED_CA_FILE |
chenraoCR
force-pushed
the
fix/bitnami/etcd/70554
branch
3 times, most recently
from
455cc83
to
9b429b0
Compare
| @@ -307,7 +307,8 @@ etcdctl_auth_norbac_flags() { | |||
| authFlags+=("--cert" "${ETCD_DATA_DIR}/fixtures/client/cert.pem" "--key" "${ETCD_DATA_DIR}/fixtures/client/key.pem") | |||
| else | |||
| [[ -f "$ETCD_CERT_FILE" ]] && [[ -f "$ETCD_KEY_FILE" ]] && authFlags+=("--cert" "$ETCD_CERT_FILE" "--key" "$ETCD_KEY_FILE") | |||
| [[ -f "$ETCD_TRUSTED_CA_FILE" ]] && authFlags+=("--cacert" "$ETCD_TRUSTED_CA_FILE") | |||
| # if CA file exists, then use CA to verify server certs; otherwise, just skip server certs verification | |||
| [[ -f "$ETCD_TRUSTED_CA_FILE" ]] && authFlags+=("--cacert" "$ETCD_TRUSTED_CA_FILE") || authFlags+=("--insecure-transport=false --insecure-skip-tls-verify=true") | |||
There was a problem hiding this comment.
Is --insecure-transport=false necessary? We didn't have it set at
There was a problem hiding this comment.
ya, can remove it, and already updated it
chenraoCR
force-pushed
the
fix/bitnami/etcd/70554
branch
2 times, most recently
from
750bb20
to
49c6d16
Compare
|
@andresbono already updated, please kindly approve this PR |
| # if CA file exists, then use CA to verify server certs; otherwise, just skip server certs verification | ||
| [[ -f "$ETCD_TRUSTED_CA_FILE" ]] && authFlags+=("--cacert" "$ETCD_TRUSTED_CA_FILE") || authFlags+=("--insecure-skip-tls-verify") | ||
| fi |
There was a problem hiding this comment.
I feel like there is some redundancy between setting --insecure-skip-tls-verify here and in the healthcheck.sh script. Do you think it can be simplified somehow?
For example, removing it from healthcheck.sh and apply the following change:
| # if CA file exists, then use CA to verify server certs; otherwise, just skip server certs verification | |
| [[ -f "$ETCD_TRUSTED_CA_FILE" ]] && authFlags+=("--cacert" "$ETCD_TRUSTED_CA_FILE") || authFlags+=("--insecure-skip-tls-verify") | |
| fi | |
| fi | |
| # If CA file exists, then use CA to verify server certs; otherwise, just skip server certs verification | |
| [[ -f "$ETCD_TRUSTED_CA_FILE" ]] && authFlags+=("--cacert" "$ETCD_TRUSTED_CA_FILE") || authFlags+=("--insecure-skip-tls-verify") | |
| fi |
But that could be insecure. Users not setting the recommended CA file would not notice it is missing, although it is recommended.
The other option is that you only set the flag in the healthcheck.sh script, changing the condition in which it is set so it fits you use case.
|
This Pull Request has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thank you for your contribution. |
…e-way tls authentication (bitnami#70554) Signed-off-by: Chen Rao <chenrao317328@163.com>
chenraoCR
force-pushed
the
fix/bitnami/etcd/70554
branch
from
49c6d16
to
10bd302
Compare
@andresbono just updated as u suggested, please kindly approve this PR |
Description of the change
modified the script healthcheck.sh to skip tls verification to allow startup etcd with one-way tls authentication
Benefits
Possible drawbacks
Applicable issues
Additional information