-
-
Notifications
You must be signed in to change notification settings - Fork 932
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: add ko support #3653
feat: add ko support #3653
Conversation
Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Codecov Report
@@ Coverage Diff @@
## main #3653 +/- ##
==========================================
+ Coverage 83.60% 83.70% +0.09%
==========================================
Files 118 119 +1
Lines 9963 10214 +251
==========================================
+ Hits 8330 8550 +220
- Misses 1316 1338 +22
- Partials 317 326 +9
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Deploying with Cloudflare Pages
|
hey @developer-guy, can you take a look? especially the docs (https://feature-2556.goreleaser.pages.dev/customization/ko/) |
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.
This looks great! Thanks for keeping this moving. Just a few smallish comments.
|
||
```yaml | ||
# .goreleaser.yaml | ||
kos: |
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.
Can this be ko:
instead?
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.
it's pluralized because it can be multiple KO configurations... is the pluralization a problem? If it is, I can change...
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.
We could build an OCI image with ko for different binaries built from the same repository, which is why I designed it that way.
@@ -916,6 +935,7 @@ type Project struct { | |||
Brews []Homebrew `yaml:"brews,omitempty" json:"brews,omitempty"` | |||
AURs []AUR `yaml:"aurs,omitempty" json:"aurs,omitempty"` | |||
Krews []Krew `yaml:"krews,omitempty" json:"krews,omitempty"` | |||
Kos []Ko `yaml:"kos,omitempty" json:"kos,omitempty"` |
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.
Kos []Ko `yaml:"kos,omitempty" json:"kos,omitempty"` | |
Ko []Ko `yaml:"ko,omitempty" json:"ko,omitempty"` |
} | ||
|
||
if ko.BaseImage == "" { | ||
ko.BaseImage = chainguardStatic |
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.
We should try to reuse whatever const ko defines upstream, in case we ever change the default base image in a future ko release.
Luckily @developer-guy has already started toward that in ko-build/ko#904, which we can reuse here when ko.BaseImage
is not defined by the user.
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.
yep, totally agree
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.
not merged/released yet, right? we can change this in the future then
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.
yep, it is not released yet, it is waiting for merge right now
if len(ko.Platforms) == 0 { | ||
ko.Platforms = []string{"linux/amd64"} | ||
} | ||
|
||
if len(ko.Tags) == 0 { | ||
ko.Tags = []string{"latest"} | ||
} | ||
|
||
if ko.SBOM == "" { | ||
ko.SBOM = "spdx" | ||
} |
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 think these should also be defaults that ko maintains upstream. If we ever change them in the future we don't want to have to go hunting for other places these have been set downstream.
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.
cool, I can PR those there too
Co-authored-by: Jason Hall <jason@chainguard.dev>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
I don't understand why it could succeed... |
Signed-off-by: Batuhan Apaydın <batuhan.apaydin@trendyol.com>
I found the problem and fixed it with cd78015. PTAL @caarlos0 |
ahh good catch @developer-guy , I knew it must be something "easy" that I was missing |
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
I've open a PR to fix this problem in ko project side: ko-build/ko#932 |
I did a few changes in your commit too, let me know what you think @developer-guy |
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
update docs, fixed the main (was being ignored :kek:), I think we are good to merge now! |
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
continuing the PR by @developer-guy
Default
method does not workfromConfig
thing should probably be on the defaults, too--skip-ko
closes #2556
closes #3490