Skip to content

feat: Add the sm-k6-gsm binary for secrets #1234

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

Merged
merged 2 commits into from
Mar 14, 2025
Merged

feat: Add the sm-k6-gsm binary for secrets #1234

merged 2 commits into from
Mar 14, 2025

Conversation

d0ugal
Copy link
Contributor

@d0ugal d0ugal commented Mar 6, 2025

This depends on grafana/xk6-sm#75 and therefore the release: grafana/xk6-sm#74

To support secrets in the short term we need to add a second k6 binary which tracks the upstream secrets work. This means it is an unreleased k6 version.

This binary will only be used when secret source configuration is provided from the API. This will start to happen with #1179

@d0ugal d0ugal marked this pull request as ready for review March 12, 2025 09:42
@d0ugal d0ugal requested a review from a team as a code owner March 12, 2025 09:42
@d0ugal d0ugal requested review from mem and nadiamoe March 12, 2025 09:42
@@ -24,6 +24,7 @@ ARG HOST_DIST=$TARGETOS-$TARGETARCH
RUN adduser -D -u 12345 -g 12345 sm

ADD --chown=sm:sm --chmod=0500 https://github.com/grafana/xk6-sm/releases/download/v0.0.3-pre/sm-k6-${TARGETOS}-${TARGETARCH} /usr/local/bin/sm-k6
ADD --chown=sm:sm --chmod=0500 https://github.com/grafana/xk6-sm/releases/download/v0.4.0/sm-k6-${TARGETOS}-${TARGETARCH}-gsm /usr/local/bin/sm-k6-gsm
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Upgrading this past the above binary is intentional. These will be out of sync for a while until the default binary is updated and then we can drop the second one.

Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure this is intended, renovate will update both URLs:

"https://github.com/grafana/xk6-sm/releases/download/(?<currentValue>[^/]+)/"

Comment on lines +14 to +20
$(DISTDIR)/$(1)-$(2)/sm-k6-gsm:
mkdir -p "$(DISTDIR)/$(1)-$(2)"
# Renovate updates the following line. Keep its syntax as it is.
curl -sSL https://github.com/grafana/xk6-sm/releases/download/v0.4.0/sm-k6-$(1)-$(2)-gsm -o "$$@"
chmod +x "$$@"

sm-k6: $(DISTDIR)/$(1)-$(2)/sm-k6 $(DISTDIR)/$(1)-$(2)/sm-k6-gsm
Copy link
Member

@nadiamoe nadiamoe Mar 14, 2025

Choose a reason for hiding this comment

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

In practice, this will only be used by the pipeline that packages the agent for Debian/Fedora (and derivatives). Container images rely on the ADD instruction (which you added).

However, since you haven't added that file to nfpm:

# Copy k6 as sm-k6 to prevent clashing with k6 if it's installed.

This means this is effectively ignored. make package* will download this but it won't be included in any package. If we do not intend to ship this in linux packages, I'd suggest getting rid of the make-related changes. If we do want to ship this binary in linux packages, then we should edit the nfpm config.

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 spot, thanks. Adding this to images only is probably enough for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although, that could become problematic so its probably better to add it everywhere 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated, hopefully I understood correctly

Copy link
Member

@nadiamoe nadiamoe left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -24,6 +24,7 @@ ARG HOST_DIST=$TARGETOS-$TARGETARCH
RUN adduser -D -u 12345 -g 12345 sm

ADD --chown=sm:sm --chmod=0500 https://github.com/grafana/xk6-sm/releases/download/v0.0.3-pre/sm-k6-${TARGETOS}-${TARGETARCH} /usr/local/bin/sm-k6
ADD --chown=sm:sm --chmod=0500 https://github.com/grafana/xk6-sm/releases/download/v0.4.0/sm-k6-${TARGETOS}-${TARGETARCH}-gsm /usr/local/bin/sm-k6-gsm
Copy link
Member

Choose a reason for hiding this comment

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

Just to make sure this is intended, renovate will update both URLs:

"https://github.com/grafana/xk6-sm/releases/download/(?<currentValue>[^/]+)/"

@d0ugal d0ugal merged commit 94a3d29 into main Mar 14, 2025
5 checks passed
@d0ugal d0ugal deleted the xk6-sm-gsm branch March 14, 2025 15:55
@sm-release-app sm-release-app bot mentioned this pull request Mar 14, 2025
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.

2 participants