search

kubernetes-sigs / kustomize

Customization of kubernetes YAML configurations
Apache License 2.0
10.7k stars 2.22k forks source link

Pin kustomize image in cockroachdb example #5691

Closed karlkfi closed 2 weeks ago

karlkfi commented 1 month ago

This PR supersedes https://github.com/kubernetes-sigs/kustomize/pull/5534

karlkfi commented 1 month ago

/assign @koba1t

karlkfi commented 1 month ago

Friendly ping: @koba1t

Does this meet your expectations of what we discussed in https://github.com/kubernetes-sigs/kustomize/pull/5534 ?

karlkfi commented 1 month ago

Still need an approval.

koba1t commented 1 month ago

@karlkfi https://github.com/kubernetes-sigs/kustomize/pull/5534

I'm dealing with a bit of security theater here. I've got a scanner and policy that wants not just a pinned version but also some kind of checksum to confirm that the version is exactly the content you expected.

Okay, I think I understand what you want. Your security scanner scans all files in this repo and detects example Yaml files in the examples/ dir, doesn't it?

I care that I think we can't block any PR that adds changes that will be detected by your scanner.

karlkfi commented 1 month ago

In this case, the yaml files aren't the issue. The internal scanner is looking at dockerfiles and checking for "go install" calls to ensure it's not just pulling the latest from the internet without a checksum. The way to add a checksum is either to embed a go.mod file with all the checksums of the dependencies OR pin the from image in the dockerfile to a specific image checksum. That way, you can be sure you're getting what you asked for and not some poisoned code from a compromised source/image repo.

The scanner isn't public, so I can't share it for CI here. And it's not something I wrote or control. It's just company policy for repos like this that we build and ship to customers.

I could alternatively add some config file to mark the examples as ignored by the scanner, but I figured you might want your examples to exemplify good security practices.

I don't think it's all that important for the examples to be updated for every version here, but if you want that, you could add a CI script to check.

koba1t commented 2 weeks ago

@karlkfi

In this case, the yaml files aren't the issue. The internal scanner is looking at dockerfiles and checking for "go install" calls to ensure it's not just pulling the latest from the internet without a checksum

I'm so sorry. I only memorized that your problem is not in the *.go file.

I don't think it's all that important for the examples to be updated for every version here, but if you want that, you could add a CI script to check.

Yes, I feel your change improves security and shows better practice. I can agree to merge this PR. I only care about what happens after we merge your PR. I think we might not be able to notice if any PRs are opened that do not set a hash in the Dockerfile because we didn't add any blockers in CI. (Although I feel almost never happens to add a new Dockerfile.)

If you already understand this, we can merge this PR.

karlkfi commented 2 weeks ago

I think we might not be able to notice if any PRs are opened that do not set a hash in the Dockerfile because we didn't add any blockers in CI.

That's fine for now. If it fails our internal scan my team will be notified and have to come back to make another PR. But it hasn't happened enough times to need automating yet, as long as future PRs don't take as long as this one has to merge.

koba1t commented 2 weeks ago

That's fine for now.

That's good!

as long as future PRs don't take as long as this one has to merge.

Yes, I feel so sorry to take a long time. :pensive:

koba1t commented 2 weeks ago

/approve

k8s-ci-robot commented 2 weeks ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karlkfi, koba1t

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/kubernetes-sigs/kustomize/blob/master/OWNERS)~~ [koba1t] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment