reconcile service account call SCI #189
Conversation
samos123
commented
Aug 11, 2023
•
samos123
commented
- Call sci.BindIdentity to allow a K8s Service account to impersonate an identity
- Created a FakeSCIControllerClient to be able to run tests easily
|
Let's make sure tests pass before merging |
internal/cloud/common.go
Outdated
| ) | ||
|
|
||
| type Common struct { | ||
| ClusterName string `env:"CLUSTER_NAME" validate:"required"` | ||
| ArtifactBucketURL *BucketURL `env:"ARTIFACT_BUCKET_URL,noinit" validate:"required"` | ||
| RegistryURL string `env:"REGISTRY_URL" validate:"required"` | ||
| Principal string `env:"PRINCIPAL"` |
There was a problem hiding this comment.
I think this should be required (validate:"required")?
There was a problem hiding this comment.
the reason I made this optional is because that would require us to always set it, even when you use something like kind or when you run the tests. I had it as validate:"required" at first but after running tests, I thought it would be better to make it optional.
There was a problem hiding this comment.
After refactoring the PR, I think this could be changed back to required. Do you think that's the right move here? Even though Kind won't need it? There might be other environments where workload identity isn't available.
There was a problem hiding this comment.
I see the benefit of requiring this (production environments need it) outweighing the negatives… dev envs needing to pass a stub value… alternative would be to have some custom validation logic, but prob not worth the effort there
samos123
force-pushed
the
sa-reconciler-use-sci
branch
from
ca9b16d
to
d8d2017
Compare
There was a problem hiding this comment.
Nice work on these!
