Fix some issues from #613 #615
Conversation
Signed-off-by: Manuel Mendez <[email protected]>
Signed-off-by: Manuel Mendez <[email protected]>
This way make can generate them in parallel if possible. Signed-off-by: Manuel Mendez <[email protected]>
This way we won't have out dated k8s manifests. Also regenerate the manifests. Signed-off-by: Manuel Mendez <[email protected]>
Signed-off-by: Manuel Mendez <[email protected]>
We shouldn't be patching a git tracked file as part of the build because then we'd have to go back and checkout the changes. Unfortunately this means we need to copy the whole directory tree so that kustomize will pick up the edited files. I also fixed a typo that used the TINK_SERVER_IMAGE_NAME for the manager. I also cleaned up the recursive uses of make since they are considered harmful and dropped `release` as that should be for the whole project not just the k8s manifests. Signed-off-by: Manuel Mendez <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #615 +/- ##
=======================================
Coverage 44.37% 44.37%
=======================================
Files 61 61
Lines 3491 3491
=======================================
Hits 1549 1549
Misses 1858 1858
Partials 84 84 Continue to review full report at Codecov.
|
| @@ -35,59 +11,55 @@ generate-go: bin/controller-gen bin/gofumpt # Generate Go code. | |||
| gofumpt -w -s ./pkg/apis | |||
|
|
|||
| .PHONY: generate-manifests | |||
| generate-manifests: bin/controller-gen # Generate manifests e.g. CRD, RBAC etc. | |||
| generate-manifests: generate-crds generate-rbacs generate-server-rbacs # Generate manifests e.g. CRD, RBAC etc. | |||
There was a problem hiding this comment.
want this to be a doc?
| generate-manifests: generate-crds generate-rbacs generate-server-rbacs # Generate manifests e.g. CRD, RBAC etc. | |
| generate-manifests: generate-crds generate-rbacs generate-server-rbacs ## Generate manifests e.g. CRD, RBAC etc. |
There was a problem hiding this comment.
Hmm good catch this was from before I just tacked it onto generate.
There was a problem hiding this comment.
Oh wait I misread the diff. No I don't think its necessary, make generate will do the right thing. If you'd like it to be documented I have no problem with it showing up in make help. Do you want this to be a doc?
There was a problem hiding this comment.
Ha It doesn't matter to me. Since this isn't a top level target, I'm fine with keeping it not doc'd
There was a problem hiding this comment.
Yeah thats been my thinking.
Description
#613 introduced a few unwanted bits and a typo.
Why is this needed
#613 did not use tools.go setup we have for using pinned go tools.
I reworked it so it also didn't do recursive make as that is considered harmful and setup better dependencies so that we let make be better equipped to do the right thing. This way we avoid using make as a dumb command runner, which if not done can lead to racy/broken makefiles (see hook for example).
I fixed a typo that would cause the manager to use tink-server image, likely to not work :D.
In the process I noticed that we don't have
generate-manifestshooked up to be run as part of ci to detect drift, so I added it as a dependency ofgeneratedas it should have been from the beginning.How Has This Been Tested?
I ran make a bunch.