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

feat: allow ssh-credentials to be set in agent and plugin #248

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

Conversation

42atomys
Copy link
Contributor

@42atomys 42atomys commented Feb 8, 2024

Description

This pull request introduces the capability to configure ssh credentials directly in the Buildkite agent and plugin settings. This feature enhances security and flexibility by allowing users to set up git access credentials, improving the management and use of private repositories in CI/CD workflows.

This pull request put gitFromEnv as deprecated because this is not relevante to the usage (currently this field is used to set a ssh credentials and can be also used as envFrom override). The gitFromEnv still working for backward compatibility but as the project are not stable yet, what is the vision about deprecated lifecycle ?

⚠️ DEPRECATED ⚠️

The method for setting git credentials in Buildkite agents stack and k8s plugins is changing.
Please update your configurations to use the new method introduced in PR #248 for enhanced security and flexibility.

# DEPRECATED WAY
gitEnvFrom:
- secretRef: { name: agent-stack-k8s }
# REPLACEMENT
ssh-credentials-secret: agent-stack-k8s

@42atomys 42atomys changed the title feat: allow git-credentials to be set in agent and plugin feat: allow ssh-credentials to be set in agent and plugin Feb 8, 2024
@42atomys 42atomys marked this pull request as ready for review February 8, 2024 10:54
@42atomys
Copy link
Contributor Author

42atomys commented Feb 8, 2024

Hi @triarius, We start the pod spec at the agent level with the ssh-credentials, other pull request will come today with the pod-spec-patch feature 🚀

I have multiple question about the future of this agent-k8s-stack and the vision to be align with it, can we discuss via Slack ?

@triarius
Copy link
Contributor

triarius commented Feb 9, 2024

Hi @42atomys, sure, but our timezones might make synchronous conversation hard. It might be best to email support@buildkite.com, and they can arrange some channel for us to communicate.

@triarius
Copy link
Contributor

@42atomys
Copy link
Contributor Author

42atomys commented Feb 12, 2024

@triarius I will send a message today to support@buildkite.com and sorry about the errors, a stash still on my computer. My bad

Currently on the last failling test

@42atomys
Copy link
Contributor Author

@triarius Trying to run integration test in an empty organization, but job is not taked by the agent, I dont found the source. I found an issue and resolve it. Sorry for the test approval train 🙏

@peterkracik
Copy link

this would be really helpful 🙏

Copy link
Contributor

@triarius triarius left a comment

Choose a reason for hiding this comment

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

@42atomys I found some time to look at this again. I think its mostly a much-needed improvement. But I have a few quibbles:

  1. The tests were failing because there was a mix-up between the kebab case needed for ssh-credentials-secret when it's in the config (and the helm values.yaml) and the camel case needed when it's in the Buildkite Pipeline YAML. I've suggested a fix, but it seems this situation is rife for confusion in the future.

  2. I'm not sure if we want to specify checkout secrets at the step level at all. Every step in a pipeline will typically need the same git secret, as every step will typically checkout the same git repo.

Given these, I'm inclined to drop the ability to specify sshCredentialsSecret in the kubernetes plugin, and just rely on what's in the controller config.

I've made a branch where I've built on your changes and obtained a green build: main...triarius/allow-git-credentials-as-secret-from-agent-and-plugin

If you're happy for me to push this to your PR branch, I'm happy to merge this as is.

Otherwise, LMK if you need the per-step sshCredentialsSecret. I think there's already at least 2 ways to get this into a containers, so it's not needed to add a third, especially as I think it's not that useful to do it on a per-step basis.

queue: {{.queue}}
plugins:
- kubernetes:
ssh-credentials-secret: agent-stack-k8s
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ssh-credentials-secret: agent-stack-k8s
sshCredentialsSecret: agent-stack-k8s

queue: {{.queue}}
plugins:
- kubernetes:
ssh-credentials-secret: agent-stack-k8s
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
ssh-credentials-secret: agent-stack-k8s
sshCredentialsSecret: agent-stack-k8s

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