-
Notifications
You must be signed in to change notification settings - Fork 189
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
Client GSSENC Request Fix #797
base: main
Are you sure you want to change the base?
Conversation
Just implemented the automated test with the most recent commit. The test simply tries to connect to the database after a valid kerberos ticket has been made available. Can confirm that this test fails without this patch and passes with it. |
Can you please run
and commit any required changes. |
Done |
I believe that these tests ran locally because the environment created by start_test_env.sh runs on root. It looks like this is not the case with the circlci environment and thus it fails when it tries to write some of the kerberos config files into root space. Further I had to add a few packages to the test docker container but it looks like the circlecli config.yaml does not build the container from scratch but uses the image ghcr.io/postgresml/pgcat-ci:latest. Given this I am not sure how to make this new test pass CI on my end. Maybe this would best be a local only test? Let me know how you want to proceed. |
Maybe you can create a PR that updates the pgcat-ci docker image with the changes you want (https://github.com/postgresml/pgcat/blob/main/Dockerfile.ci) and merge that PR Perhaps you can make Sorry, for the hassle. The workflow can be improved and I am trying to reduce the friction for future contributors so your help is appreciated! |
I think what you are saying makes a lot of sense. Will see what I can do. |
Looking through the repo there are currently 5 different docker files and 3 different docker compose files. In particular it seems like there is a lot of overlap between the docker assets in Also its seem like Also not seeing any reference to Also any reason why go is not installed in $ find . | grep docker-compose.yml
dev/docker-compose.yaml
docker-compose.yml
tests/docker/docker-compose.yml find . | grep Docker
Dockerfile
Dockerfile.ci
Dockerfile.dev
dev/Dockerfile
tests/docker/Dockerfile |
Yes, this is actually one of the things that I really wanted to fix for the longest time. For As for So the grand total should be, two I was planning to add another docker-compose file but that would be for benchmarking so I am thinking I could add a |
Agree, would definitely help DRY out the repo and improve developer experience. |
This PR fixes an issue where a client that attempts to connect to pgcat with GSS encoding is rejected with a confusing error message. By default libpq will attempt to connect with GSS encoding if there is a valid kerberos ticket available on the system which can be checked with the
klist
command. The server should reject this request at which point libpq will attempt to connect again without gss encoding. This behavior is documented in the postgres documentation.Do not have any automated tests right now, will need to set up a kerberos environment in the container. Can do this a bit later.
Fixes #792