-
Notifications
You must be signed in to change notification settings - Fork 301
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
Add logs to allowed-[repositories|plugins] #2810
Conversation
## Context A handy log line, which supports a process of enabling allowed repo and plugins on buildkite-agent. ## Intent My first attempt of enabling buildkite allowed-repo and allowed-plugins failed. I'm not entirely sure if I'm propagating the variables correctly, so this log line would be pretty useful in debugging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jakubm-canva, thanks for the PR! I have a suggestion that might make the new logs slightly more useful, WDYT?
clicommand/agent_start.go
Outdated
@@ -1032,6 +1032,7 @@ var AgentStartCommand = cli.Command{ | |||
} | |||
|
|||
if len(cfg.AllowedRepositories) > 0 { | |||
l.Info("Evaluating allowed repositories has been enabled") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if these would be better at the end of the block (after they've all been parsed - if any of them fail there will be a Fatal
log), and if they included the values (after parsing) so you know what it understood, e.g.
l.Info("Allowed repositories patterns: %q", agentConf.AllowedRepositories)
...
l.Info("Allowed plugins patterns: %q", agentConf.AllowedPlugins)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me. Let me rework it and update the PR.
FYI. I recall you from SYD-ODI. You were working with Marek 😉 He was my 🇵🇱 connection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! Hey 👋 I thought you looked familiar. I do miss my old colleagues a bit 😔 How long have you been at Canva?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After nearly 7 years in network engineering and 1.5 year as embedded SWE in ChromeOS, the time has come to make a change. Working on Infra / CI since Apr, 2023 and I'm having good fun 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, glad you're having fun! I took some time off between exiting SRE and coming to BK in Oct 2022, it was worth it.
Description
A handy log line, which supports a process of enabling allowed repo and plugins on buildkite-agent.
Context
My first attempt of enabling buildkite allowed-repo and allowed-plugins failed. I'm not entirely sure if I'm propagating the variables correctly, so this log line would be pretty useful in debugging.
Changes
Testing
go test ./...
). Buildkite employees may check this if the pipeline has run automatically.go fmt ./...
)