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

Allow running the annotate script outside of Docker #239

Conversation

erict-square
Copy link
Contributor

Why

This might be a weird request, but I'm curious if it's reasonable to provide a non Docker in Docker (dind) option to run this junit-annotate-buildkite-plugin?

We have a usecase where we don't have sysbox in the container's runtime environment and there isn't a secure way to run dind (We used to mount docker socket but we've removed it due to the the nature of the sensitive workload). However, we still want to use the plugin. Perhaps it makes sense to add an additional option, assuming that Ruby is in the runtime, to run the junit xml analysis and annotation in the runtime environment w/o dind.

**Why**

This might be a weird request, but I'm curious if it's reasonable to provide a non Docker in Docker (dind) option to run this junit-annotate-buildkite-plugin?

We have a usecase where we don't have sysbox in the container's runtime environment and there isn't a secure way to run dind (We used to mount docker socket but we've removed it due to the the nature of the sensitive workload). However, we still want to use the plugin. Perhaps it makes sense to add an additional option, assuming that Ruby is in the runtime, to run the junit xml analysis and annotation in the runtime environment w/o dind.
Copy link
Contributor

@toote toote left a comment

Choose a reason for hiding this comment

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

great idea! just a minor comment with a superfluous test, but it can be merged as-is

Comment on lines 532 to 558
@test "runs in docker by default" {
export BUILDKITE_PLUGIN_JUNIT_ANNOTATE_ARTIFACTS="junits/*.xml"
export BUILDKITE_PLUGIN_JUNIT_ANNOTATE_FAIL_BUILD_ON_ERROR=false

stub mktemp \
"-d \* : mkdir -p '$artifacts_tmp'; echo '$artifacts_tmp'" \
"-d \* : mkdir -p '$annotation_tmp'; echo '$annotation_tmp'"

stub buildkite-agent \
"artifact download \* \* : echo Downloaded artifact \$3 to \$4" \
"annotate --context \* --style \* : cat >'${annotation_input}'; echo Annotation added with context \$3 and style \$5, content saved"

stub docker \
"${DOCKER_STUB_DEFAULT_OPTIONS} ruby /src/bin/annotate /junits : echo '<details>Failure</details>' && exit 64"

run "$PWD/hooks/command"

assert_success

assert_output --partial "Annotation added with context junit and style error"
assert_equal "$(cat "${annotation_input}")" '<details>Failure</details>'

unstub mktemp
unstub buildkite-agent
unstub docker
rm "${annotation_input}"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

while not against it, this test is implied by every other test that does not include the BUILDKITE_PLUGIN_JUNIT_ANNOTATE_RUN_IN_DOCKER environment variable

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call! I just removed the test!

@mcncl mcncl merged commit 6d906ad into buildkite-plugins:master Dec 10, 2024
1 check passed
@erict-square
Copy link
Contributor Author

@mcncl @toote Thank you so much for the quick reviews! I'm not familiar with the release process. When can we expect a new release to be out with this change?

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.

3 participants