Upgrade project to kubebuilder v3 #46
Conversation
|
Fixes #35 |
|
Fixes #22 |
soellman
changed the title
|
Ready for review @rosmo (especially the TF pieces) |
terraform/kubernetes/main.tf
Outdated
| @@ -251,24 +299,44 @@ resource "kubernetes_deployment" "deployment_autoneg_controller_manager" { | |||
|
|
|||
| spec { | |||
| service_account_name = kubernetes_service_account.service_account.metadata[0].name | |||
| automount_service_account_token = true | |||
There was a problem hiding this comment.
Was this removed because terraform now defaults the value to true?
While technically not required, it does seem nice to explicitly assert that the service account token is used by this workload. Terraform has already changed this default once (from false which is more secure to true which matches the kubernetes default) and kubernetes wants to change the default to false since it's more secure but hasn't done so because it would be a breaking change.
There was a problem hiding this comment.
I removed this to match the kubebuilder autogenerated manifests more closely. That said, I'm happy to consider particular cases.
For posterity, I'll sat that these changes were made by hand, and any exceptions we have here could conceivably get overwritten if/when we'd have some automated tooling to do the conversion.
That said, I'll defer to you - would you like to see an explicit statement here affirming the default of true for automount_service_account_token ?
There was a problem hiding this comment.
The default in the terraform kubernetes provider was changed in 2.0.0:
https://github.com/hashicorp/terraform-provider-kubernetes/releases/tag/v2.0.0
This either needs to require >= 2.0.0 (doesn't look like it sets a min version?) or explicitly set the value to true to support earlier versions.
There was a problem hiding this comment.
I've readded this explicit declaration.
There was a problem hiding this comment.
@rwkarg PTAL and resolve if you're satisfied
soellman
force-pushed
the
kubebuilder3
branch
from
a23294a
to
bbccd54
Compare
|
@soellman Could you rebase? Thank you :) |
|
@soellman Would you mind signing the CLA again with your current email and letting me know what that is so I can amend the commits to include your new email address. |
rosmo
force-pushed
the
kubebuilder3
branch
3 times, most recently
from
948ae48
to
dec4816
Compare
|
Fixes #75 |
Add full Terraform testing example (not E2E yet though) to validate Autoneg setup easier.
Updates to kubebuilder 3.1.0.
kubebuilder 3.1.0 was used to create a new repo, made changes the new repo, and then merged back into the existing repo. Any non-cosmetic changes were performed to prefer the standard scaffolding in v3.
One change of note: the k8s service account used is now
autoneg-system:autoneg-controller-managerinstead ofautoneg-system:default.README.mdanddeploy/workload-identity.shreflect that change.Terraform updates have not yet been made - do not merge until this is complete.