-
Notifications
You must be signed in to change notification settings - Fork 1
feat: adding service account as possible creds #21
Conversation
fb7f1f5
to
58c8057
Compare
58c8057
to
dd680b8
Compare
Signed-off-by: Piaras Hoban <phoban01@gmail.com>
… successfully reconciliation Signed-off-by: Piaras Hoban <phoban01@gmail.com>
pkg/ocm/ocm.go
Outdated
if err != nil { | ||
return false, fmt.Errorf("failed to get repository for spec: %w", err) | ||
panic(err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: remove
be639f5
to
c4ff2e8
Compare
c4ff2e8
to
498f2da
Compare
We'll test that with the end-to-end flow. It's covered now partially through unit tests. |
It's workiiiing! :)
Previously, when we deleted the usage of the service account it was still authenticated because the context remained configured. Now it's no longer doing that. |
adb47b8
to
de89424
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small nits; tests are really great.
pkg/ocm/ocm.go
Outdated
} | ||
} | ||
|
||
if err := c.maybeConfigureAccessCredentials(ctx, octx, obj.Spec.Source, obj.Namespace); err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is kind of a weird name 😄 configureCredentials
would make sense to me.
secretRef: | ||
name: creds | ||
credentials: | ||
secretRef: | ||
name: creds | ||
url: ghcr.io/phoban01/ocm-podify | ||
destination: | ||
secretRef: | ||
name: creds | ||
credentials: | ||
serviceAccountName: service-account-for-destination |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this needs to be updated I think
pkg/ocm/ocm.go
Outdated
if obj.Spec.ServiceAccountName == "" { | ||
return nil | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could this check be outside the method? then you only need to pass in the service account name and the contexts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also using obj.Namespace,
. But I can pass that in instead. Would minimize the memory burden.
}, | ||
}, | ||
{ | ||
name: "component access with secret ref", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
efe7f28
to
f3e030e
Compare
No description provided.