🐛 fix: ignore non-go files during Docker build

[busser]

This change ensures that the scaffolded .dockerignore file makes Docker ignore non-Go files.

This avoids large files like test binaries being unnecessarily included in Docker's build context.

Fixes #2073.

[k8s-ci-robot]

Welcome @busser!

It looks like this is your first PR to kubernetes-sigs/kubebuilder 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/kubebuilder has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @busser. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[camilamacedo86]

We need to check if it will work when is multigroup when is not since it means that it can be:
https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v3-multigroup/controllers
OR
https://github.com/kubernetes-sigs/kubebuilder/tree/master/testdata/project-v3/controllers

Same regards with api.

[busser]

Good call.

I just tested it and it does not work. Further investigation lead me to this issue: moby/moby#30018. Basically, we can't write a whitelist-style .dockerignore file.

Should we opt for a different strategy, like ignoring directories that are known to contain large files that are unnecessary when building a Docker image for the operator? The .dockerignore file would look like this:

# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
# Ignore build and test binaries.
bin/
testbin/

[camilamacedo86]

Hi @busser,

See that the idea to ignore any file which is not a GO type. Then, let's check the docker docs:

Beyond Go’s filepath.Match rules, Docker also supports a special wildcard string ** that matches any number of directories (including zero). For example, **/*.go will exclude all files that end with .go that are found in all directories, including the root of the build context.

https://docs.docker.com/engine/reference/builder/#dockerignore-file

So, according to the docker documentation !**/*.go is correct which means that we are generating the file right. In this way, could you please review your test and see if you are not missing anything?

Other options are:

If the previous suggestion works when is not multigroup then, see that we can:

• (option A) duplicate that to add the respective paths for multigroup by default as well
• (option B) we can change the file as we change the ignoredockerfile when we enable the multi-group support
• (option C) we can check a syntax that will ignore both paths. (I think it is the best solution)
• (option D) Currently, ignore any file which is not a GO type. (that would be the best, but if we have a problem with ! then, we also need to find or track an issue to docker to pin that here.)

[busser]

Hi @camilamacedo86,

The pattern we currently use, !**/*.go should work according to the Docker docs, but in fact doesn't. My understanding is that we are facing a bug in how Docker handles .dockerignore logic. This issue (moby/moby#30018) reports a three year-old bug in Docker: when a pattern starts with !, wildcard strings like * and ** don't work for directories.

I don't believe this will ever be fixed. The Docker team's stated long-term goal is for .dockerignore to have the same behaviour as .gitignore. I found this section in the .gitignore specification:

It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined.

So I guess that we won't ever be able to exclude the entire working directory except for GO files... 😞

[busser]

In its current state, the .dockerignore file scaffolded by Kubebuilder ignores nothing and includes the entire directory in the Docker build context. Most of the files are just code and are relatively small. However, the Kubebuilder workflow adds large binaries to the bin/ and testbin/ directories. I would recommend ignoring these directories by default and letting the user add more paths to their .dockerignore based on their specific needs.

What do you think?

[camilamacedo86]

Would not the following configuration solve the need for multi-group and not multi-group layout projects?

!go.mod
!go.sum
!main.go
!controllers/**/*.go
!api/**/*.go
!controllers/*.go
!api/*.go

[camilamacedo86]

HI @busser,

After checking it better I think the best solution is the one suggested by you:

# More info: https://docs.docker.com/engine/reference/builder/#dockerignore-file
# Ignore build and test binaries.
bin/
testbin/

[Adirio]

What about the config directory? We should probably add a marker so that plugins can add their own exceptions, and that way the config plugin would be the one adding that line.

[busser]

@camilamacedo86 I've updated the pull request to ignore the build and test binaries, as decided above 👍

[busser]

@Adirio How would you go about doing this? I don't mind doing the implementation myself, but would need some guidance 🙂

[camilamacedo86]

/ok-to-test

It seems ok for me in order to solve the issue.

@Adirio makes the following suggestion:

What about the config directory? We should probably add a marker so that plugins can add their own exceptions, and that way the config plugin would be the one adding that line.

That is fine but seems an RFE. So, @Adirio wdyt about we move forward here and then raise that as an issue for we are able to track and work on that too?

HI @estroz @Adirio,

It is open for a while. WDYT about we move forward here and do any improvement and suggestion as a follow-up?

[camilamacedo86]

/lgtm
I think it is fine.
@Adirio please feel free to raise any issue if you see that it requires a follow up for improvements.

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: busser, camilamacedo86

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 [camilamacedo86]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment