-
Notifications
You must be signed in to change notification settings - Fork 149
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
server,ui: infinite HTTP redirect when accessing UI for secure cluster behind nginx-ingress in k8s #228
server,ui: infinite HTTP redirect when accessing UI for secure cluster behind nginx-ingress in k8s #228
Comments
Hello, I am Blathers. I am here to help you get the issue triaged. Hoot - a bug! Though bugs are the bane of my existence, rest assured the wretched thing will get the best of care here. I have CC'd a few people who may be able to assist you:
If we have not gotten back to your issue within a few business days, you can try the following:
🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is otan. |
53991: pgwire: accept non-TLS client conns safely in secure mode r=aaron-crl,irfansharif,bdarnell a=knz Fixes #44842. Informs #49532. Informs #53404. This change makes it possible for a DBA / system administrator to reconfigure individual nodes *in a secure cluster* to accept SQL client sessions over TCP without mandating a TLS handshake. Authentication remains mandatory as per the HBA rules. Motivation: we have at least two high-profile customers who keep their nodes and client apps in a private secure network (with network-level encryption / privacy) and who experience client-side TLS as unnecessary and expensive friction. Additionally, **this feature is a prerequisite to upgrade an insecure cluster to secure mode without downtime.** Why this does not impair security: - authentication remains mandatory (as per the HBA rules -- [1] [2]). - the feature is opt-in: the operator must set a command-line flag (`--accept-sql-without-tls`), which is not enabled by default. - there is an interlock: the user must both set up the flag and set log-in passwords for their SQL users (by default, users get created without a password and thus cannot log in without client certs). - for now, node-node connections still require TLS. [1]: https://www.postgresql.org/docs/12/auth-pg-hba-conf.html [2]: https://dr-knz.net/authentication-in-postgresql-and-cockroachdb.html For context, the default HBA configuration is the following: ``` host all root all cert-password # fixed rule host all all all cert-password # built-in CockroachDB default local all all password # built-in CockroachDB default ``` The directive `host` covers both TLS and non-TLS incoming TCP connections (`local` is for the unix socket). The method `cert-password` means "client cert or password": without a cert, the password is mandatory. As previously, the user can further secure the configuration by restricting non-TLS connections to just a subnetwork, for example: ``` host all all 10.0.0.0/8 password # accept conns on the 10/8 network. host all all all reject # refuse conns from other nets. local all all password ``` Note that this change is limited to the server side: CockroachDB's own `cockroach` CLI commands do not yet know how to connect to a CockroachDB server without TLS; such connections are only supported from `psql` or SQL client drivers in apps. See #53994 for a follow-up. Release justification: fixes for high-priority or high-severity bugs in existing functionality 54019: roachtest: de-flake 'inconsistent' r=knz a=tbg This test sets up an intentionally corrupted replica and wants its node to shut down as a result of its detection. When only two of the three nodes were included in the consistency check, either one of them could end up terminating (as no obvious majority of healthy replicas can be determined). Change the test so that we wait for the cluster to come fully together before setting a low consistency check interval. Closes #54005. Release justification: testing Release note: None Co-authored-by: Raphael 'kena' Poss <[email protected]> Co-authored-by: Tobias Grieger <[email protected]>
cc @dhartunian this seems to fall between the cracks of server / obs infra. What to do? |
@knz any updates on this issue? |
We've not found the direct cause of the issue yet, but you might have interest in cockroachdb/cockroach#74329. |
Thanks for the information, but it looks that neither of those flags will not resolve the problem with the redirect. |
@sergeyshaykhullin can you connect to the web ui from inside the cluster if you do portforwarding and bypass the nginx ingress? I assume you have something like |
@mihneastaub or @sergeyshaykhullin can you also provide your k8s nginx annotations? |
@udnay I'm using istio gateway for the ingress controller. |
metadata:
annotations:
kubernetes.io/tls-acme: "true"
meta.helm.sh/release-name: crdb
meta.helm.sh/release-namespace: crdb
nginx.ingress.kubernetes.io/auth-signin: https://$host/oauth2/start?rd=$escaped_request_uri
nginx.ingress.kubernetes.io/auth-url: https://$host/oauth2/auth
nginx.ingress.kubernetes.io/service-upstream: "true" |
While starting to take a look into this, I was able to reproduce this issue using the following steps. I figured these steps could be helpful if anyone else is interested in verifying the issue or playing around with different config settings. Start by creating a few files we'll use for getting everything set up. Support files for testing# hack/issuer.yaml
apiVersion: cert-manager.io/v1
kind: Issuer
metadata:
name: letsencrypt-staging
spec:
acme:
server: https://acme-staging-v02.api.letsencrypt.org/directory
email: [email protected]
privateKeySecretRef:
name: letsencrypt-staging
solvers:
- http01:
ingress:
class: nginx # hack/values.yaml
ingress:
enabled: true
annotations:
cert-manager.io/issuer: letsencrypt-staging
kubernetes.io/ingress.class: nginx
hosts:
- crdb.example.com
tls:
- hosts: [crdb.example.com]
secretName: crdb-example-com-tls # hack/exec (don't forget to chmod +x this file)
#!/usr/bin/env bash
set -euo pipefail
CLUSTER="${CLUSTER:-"$(whoami)-test"}"
REGION="${REGION:-us-east1}"
main() {
case "${1:-}" in
create-cluster) create_cluster;;
delete-cluster) delete_cluster;;
apply)
# ensure we've got the necessary binaries
make bin/helm bin/kubectl
install_cert_manager
install_ingress_nginx
install_chart;;
*)
echo "Unknown command '${1:-}'." 1>&2
echo "Usage: ${0} <command>"
echo " commands:"
echo " create-cluster create a GKE cluster for testing"
echo " delete-cluster delete the GKE cluster"
echo " apply (idempotently) install cert-manager, ingress-nginx, and this chart"
exit 1;;
esac
}
create_cluster() {
println "Creating GKE cluster ${CLUSTER}..."
gcloud container clusters create "${CLUSTER}" --region "${REGION}" --num-nodes 1
}
delete_cluster() {
println "Deleting GKE cluster ${CLUSTER}..."
gcloud container clusters delete "${CLUSTER}" --region "${REGION}"
}
install_chart() {
println "Installing local cockroachdb chart..."
bin/helm upgrade -f hack/values.yaml --install crdb ./cockroachdb
}
install_ingress_nginx() {
println "Installing ingress-nginx..."
bin/helm upgrade --install ingress-nginx ingress-nginx \
--repo https://kubernetes.github.io/ingress-nginx \
--namespace ingress-nginx --create-namespace
println "Waiting for ingress controller to be available..."
bin/kubectl wait --namespace ingress-nginx \
--for=condition=ready pod \
--selector=app.kubernetes.io/component=controller \
--timeout=90s
}
install_cert_manager() {
println "Installing cert-manager..."
bin/helm upgrade --install cert-manager cert-manager \
--repo https://charts.jetstack.io \
--namespace cert-manager \
--create-namespace \
--set installCRDs=true
println "Creating letencrypt issuer..."
bin/kubectl apply -f hack/issuer.yaml
}
println() {
echo -e "\033[36m${1}\033[0m"
}
main "$@"
Install cert-manager, ingress-nginx, and the cockroachdb chart by running With everything set up, get a hold of the external IP for the ingress object:
cURL the ingress and verify you get an error about maximum redirects reached:
|
I think this behavior is due to this snippet below which redirects all traffic on the HTTP port to HTTPS except for the healthcheck. @pseudomuto I'm guessing Maybe what we should do is inspect |
Is there any workaround for this problem? |
Any progress here? |
Waiting to see a solution for this? A lot of people use ingresses... |
Heads up for others; If your ingress is nginx based you can add the Found this solution here: https://stackoverflow.com/questions/54459015/how-to-configure-ingress-to-direct-traffic-to-an-https-backend-using-https/54459898#54459898 |
@HcgRandon |
NOPE STILL DOESNT WORK
|
@ghost apiVersion: crdb.cockroachlabs.com/v1alpha1
kind: CrdbCluster
metadata:
# this translates to the name of the statefulset that is created
name: cockroachdb
spec:
dataStore:
pvc:
spec:
accessModes:
- ReadWriteOnce
resources:
requests:
storage: "20Gi"
volumeMode: Filesystem
resources:
requests:
# This is intentionally low to make it work on local kind clusters.
cpu: 2
memory: 4Gi
limits:
cpu: 4
memory: 8Gi
tlsEnabled: true
# You can set either a version of the db or a specific image name
# cockroachDBVersion: v21.2.8
image:
name: cockroachdb/cockroach:v22.1.1
# nodes refers to the number of crdb pods that are created
# via the statefulset
nodes: 3
additionalLabels:
crdb: is-cool
# affinity is a new API field that is behind a feature gate that is
# disabled by default. To enable please see the operator.yaml file.
# The affinity field will accept any podSpec affinity rule.
# affinity:
# podAntiAffinity:
# preferredDuringSchedulingIgnoredDuringExecution:
# - weight: 100
# podAffinityTerm:
# labelSelector:
# matchExpressions:
# - key: app.kubernetes.io/instance
# operator: In
# values:
# - cockroachdb
# topologyKey: kubernetes.io/hostname
# nodeSelectors used to match against
# nodeSelector:
# worker-pool-name: crdb-workers
# Ingress for access the services
ingress:
ui:
ingressClassName: nginx
annotations:
nginx.ingress.kubernetes.io/backend-protocol: "HTTPS" <-- add here
host: some.hostname |
has anyone tried using path based routing instead of host based routing? I am facing issue on
|
@prafull01 can you triage? |
@himanshu-cockroach Can you please have a look |
@rajavarapusaiprasanth-hpe The path based routing is not working currently because of this issue: cockroachdb/cockroach#91429. since all the other paths including UI dashboard, other than the absolute paths which are being redirected to via the code, its working through path based routing, we can close this thread. |
This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: cockroachdb#91429 Epic: None Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.
This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: cockroachdb#91429 Epic: None Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.
108824: sql: implement datetime builtins r=rafiss,chrisseto a=annrpom Previously, `make_date`, `make_timestamp`, and `make_timestamptz` built-ins were not implemented. In addition, a new signature for `date_trunc` was not implemented. This was inadequate because it caused an incompatibility issue for tools that needed these datetime functions. To address this, this patch adds said datetime built-ins. Note that behaviors do not align with postgres when negative years are inpits, as we adhere to ISO 8601 standard. Epic: none Fixes: #108448 Release note (sql change): Datetime built-ins (make_date, make_timestamp, and make_timestamptz) are implemented - allowing for the creation of timestamps, timestamps with time zones, and dates. In addition, date_trunc now allows for a timestamp to be truncated in a provided timezone (to a provided precision). 109346: kvcoord: Pace rangefeed client goroutine creation r=miretskiy a=miretskiy Acquire catchup scan quota prior to goroutine creation in order to pace the goroutine creation rate. This change results in nice and smooth growth in goroutine count, thus reducing the pressure on goroutine scheduler, which in turn reduces the impact on SQL latency during changefeed startup. This change also improves observability in rangefeed client by introducing new counters: * `distsender.rangefeed.retry.<reason>`: counter keeping track of the number of ranges that ecountered a retryable error of particular type (e.g. slow counsumer, range split, etc). Observability also enhanced by adding a column to `crdb_internal.active_rangefeed` virtual table augment to indicate if the range is currently in catchup scan mode. Fixes #98842 Release note (enterprise change): Pace rangefeed goroutine creation rate to improve scheduler latency. Improve observability by adding additional metrics indicating the reason for rangefeed restart as well as additional column in the `crdb_internal.active_rangefeed` table to indicate if the range is currently in catchup scan mode. 109694: ui: all xhr paths from db console are now relative r=santamaura,zachlite,j82w a=dhartunian This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: #91429 Epic CRDB-21265 Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths. 109774: sql,keys: disregard checks in *ZoneConfig*Batch r=rafiss a=annrpom Previously, a bug occured where a transactional schema change, `ALTER RANGE default CONFIGURE ZONE...`, statement would produce a CPut failure, even though both statements do take effect. This was due to a guard in the code that blocked us from adding the first zone config to the uncommitted cache, causing the expValues being updated to the KV batch to be the same for both statements. This patch addresses the issue by removing the check and allowing for `default` and psuedotables (like system ranges) to be added to the uncommitted cache. Epic: none Release note (bug fix): two `ALTER RANGE default CONFIGURE ZONE` statements on the same line no longer displays an error. Fixes: #108253 110250: changefeedccl: Fix data race in lagging spans metric r=miretskiy a=miretskiy Fix a race bug in lagging spans metric. Fixes: #110235 Release note: None Co-authored-by: Annie Pompa <[email protected]> Co-authored-by: Yevgeniy Miretskiy <[email protected]> Co-authored-by: David Hartunian <[email protected]>
This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: cockroachdb#91429 Epic: None Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.
This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: cockroachdb#91429 Epic: None Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.
This commit removes all prefix `/` characters from request paths in the DB Console and Cluster UI codebases. This ensures that if the DB Console is proxied at a subpath the requests continue to work as expected and are relative to the correct base path. Resolves: cockroachdb/helm-charts#228 Resolves: #91429 Epic: None Release note (ops change, ui change): The DB Console now constructs client-side requests using relative URLs instead of absolute ones. This enables proxying of the DB Console at arbitrary subpaths.
Describe the problem
I am getting
too many redirects
when trying to access secure cockroachdb cluster behind ingress-nginx using helm chart.Please describe the issue you observed, and any steps we can take to reproduce it:
It is happening because tls.enabled == secure cluster with tls termination.
Same thing i got with argo cd and solved using --insecure flag (Insecure - just about tls termination)
So i have to use insecure cluster behind ingress (losing auth screen, users with passwords etc) or secure cluster without ingress inside k8s (And i have to manage certificates, use not 80 and 443 ports, it's painful)
To Reproduce
Expected behavior
I can disable tls termination in web ui, but don't lose security cluster benefits
Additional data / screenshots
Environment:
Jira issue: CRDB-4219
The text was updated successfully, but these errors were encountered: