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

PMM-13748 gssapi support #1042

Open
wants to merge 31 commits into
base: main
Choose a base branch
from
Open

PMM-13748 gssapi support #1042

wants to merge 31 commits into from

Conversation

idoqo
Copy link
Contributor

@idoqo idoqo commented Mar 5, 2025

PMM-13748

This adds support for the GSSAPI auth mechanism for the exporter by compiling the binary with the -tags gssapi flag. It also adds additional kerberos environment (via docker) for tests.

@idoqo idoqo marked this pull request as ready for review March 5, 2025 22:03
@idoqo idoqo requested a review from a team as a code owner March 5, 2025 22:03
@idoqo idoqo requested review from BupycHuk, JiriCtvrtka and ademidoff and removed request for a team March 5, 2025 22:04
Copy link
Member

@BupycHuk BupycHuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestions for improvement

Comment on lines 290 to 291
build:
dockerfile: ./docker/kerberos.dockerfile
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we have pre-built docker image, so we don' have to build it everytime

Makefile Outdated
@@ -71,7 +71,7 @@ init: ## Install linters
cd tools && go generate -x -tags=tools

build: ## Compile using plain go build
go build -ldflags="$(GO_BUILD_LDFLAGS)" -o $(PMM_RELEASE_PATH)/mongodb_exporter
env CGO_ENABLED=1 go build -ldflags="$(GO_BUILD_LDFLAGS)" -o $(PMM_RELEASE_PATH)/mongodb_exporter -tags gssapi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
env CGO_ENABLED=1 go build -ldflags="$(GO_BUILD_LDFLAGS)" -o $(PMM_RELEASE_PATH)/mongodb_exporter -tags gssapi
CGO_ENABLED=1 go build -ldflags="$(GO_BUILD_LDFLAGS)" -o $(PMM_RELEASE_PATH)/mongodb_exporter -tags gssapi

There is no need to pass all of the environment variables to Go.

"mongodb_up",
}
err = testutil.CollectAndCompare(gc, expected, filter...)
assert.NoError(t, err, "mongodb_up metric should be 1")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert.NoError(t, err, "mongodb_up metric should be 1")
require.NoError(t, err, "mongodb_up metric should be 1")

@@ -0,0 +1,4 @@
#!/bin/bash

docker exec --user root psmdb-kerberos bash -c 'chown mongodb:root /krb5/mongodb.keytab'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
docker exec --user root psmdb-kerberos bash -c 'chown mongodb:root /krb5/mongodb.keytab'
docker exec --user root psmdb-kerberos bash -c 'chown mongodb:mongodb /krb5/mongodb.keytab'

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there's no mongodb group, only the user.

password=${MONGO_INITDB_ROOT_PASSWORD}

echo "Waiting for startup.."
until mongosh --host 127.0.0.1:27017 -u ${username} -p ${password} --eval 'quit(db.runCommand({ ping: 1 }).ok ? 0 : 2)' &>/dev/null; do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will loop forever if it's unable to connect. Shall we consider a timeout or a number of retries?

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.

4 participants