-
Notifications
You must be signed in to change notification settings - Fork 251
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
Add support for service-specific service accounts in place of default
in generated kustomize manifests
#777
Add support for service-specific service accounts in place of default
in generated kustomize manifests
#777
Comments
|
Issues go stale after 90d of inactivity. |
/remove-lifecycle stale |
Double check the below commit, test and verify: |
Issues go stale after 90d of inactivity. |
/assign |
/lifecycle frozen |
Issue #, if available: - Relates: aws-controllers-k8s/community#777 Description of changes: This pull request is needed to get tests to pass for - aws-controllers-k8s/code-generator#270 It also removes `sha` information from k8's versions, since this `sha` version is not static. Meaning that k8s can and does update a sha for a given version to fix security updates and some of these `shas` can no longer be pulled locally. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Adam D. Cornett <adc@redhat.com>
Issue #, if available: - Fixes: aws-controllers-k8s/community#777 Description of changes: The goal of this is to move away from using the default service account. It seems this might have been done with the helm charts/templates, but was not ported to the Kustomization files. This means each operator/controller would have its own service account. Things that changed are below: - Refactored build so that ServiceAccountName is available in ack-generate controller process as well as ack-generate release process - added in `/rbac/service-account.yaml.tpl` to create a service account, and then referece this account in the `Deployment` and `ClusterRoleBinding` - Refactored build/release `sh` files to add in `ACK_GENERATE_SERVICE_ACCOUNT_NAME` to be used in both build and release process - Reordered some of the variables in the `sh` files so that if there is an error in the build/release process the `Default` values are actually presented to the user (currently they show up as blank since the variables are not initilaized before the `USAGE` variable) - **NOTE:** This changes the service account name in the `helm` process as well. It changes it from `ack-$SERVICE-controller` to `ack-$SERVICE-service-account`. I am not sure if this is backwards compatible for when a controller is installed/updated via a helm deployment. By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. Signed-off-by: Adam D. Cornett <adc@redhat.com>
Is your feature request related to a problem?
Yes - I ran into an issue when deploying multiple service controllers into the same
ack-system
namespace in an Operator Lifecycle Manager based cluster.OLM establishes an owner reference on service accounts that are used for installed controllers. When two controllers use the same service account, the original owner reference is removed which breaks OLM's ability to upgrade the controller.
NOTE - this is being patched on the OLM side! operator-framework/operator-lifecycle-manager#2128
However - Kubebuilder has enabled support for non-default service accounts as of v3.0.0:
https://github.com/kubernetes-sigs/kubebuilder/releases/tag/v3.0.0
and, more specifically
kubernetes-sigs/kubebuilder#2070
With that in mind, I think it may still be beneficial to add support for service-specific non-default service accounts.
Describe the solution you'd like
Existing kustomize configuration files that are generated for a controller will configure RBAC in association with the default service account. That is, (Cluster)RoleBindings will, by default, target the default service account, and the controller deployment manifest will render with no
serviceAccountName
definition, which means that it will get the default service account when applied to a cluster.Consider scaffolding these with a service account reference relative to the controller that's being generated. E.g.
ack-sagemaker-service-account
for the SageMaker controller.To implement, I think the changes would be fairly minimal:
serviceAccountName
key with a templated valueack-{{ .ServiceIDClean }}-service-account
to https://github.com/aws-controllers-k8s/code-generator/blob/main/templates/config/controller/deployment.yaml.tpl#L55name
fromdefault
toack-{{ .ServiceIDClean }}-service-account
in https://github.com/aws-controllers-k8s/code-generator/blob/main/templates/config/rbac/cluster-role-binding.yaml.tpl#L11service-account.yaml
file to the https://github.com/aws-controllers-k8s/code-generator/blob/main/templates/config/rbac/kustomization.yaml.tpl.A note that, as described, there would not be backwards compatibility with the previous
default
service account. I would assume that backwards compatibility would be a matter of adding a flag for the service account to the relevant generator (which I believe is the controller generator) and then handling the user input or default values. If no backwards compatibility is required, then you should just be able to make the above changes.Describe alternatives you've considered
I've considered making adjustments to the service account in post-generation scripts, but it overall seems like it would be a bit too flimsy.
The text was updated successfully, but these errors were encountered: