Skip to content

Commit

Permalink
Update the expand hub template access design
Browse files Browse the repository at this point in the history
Relates:
https://issues.redhat.com/browse/ACM-12129

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
  • Loading branch information
mprahl committed Jul 16, 2024
1 parent 1e552da commit fcc5fa0
Showing 1 changed file with 39 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,14 @@ func (r *ReplicatedPolicyReconciler) getToken(ctx context.Context, namespace str
expirationTimestamp := returnedTokenReq.Status.ExpirationTimestamp

go func() {
log.V(2).Info("Got a token", "serviceAccount", saName, "namespace", namespace, "expiration", expirationTimestamp.UTC().Format(time.RFC3339))

defer func() {
err := os.Remove(tokenFilePath)
if err != nil {
if err := tokenFile.Close(); err != nil {
log.Error(err, "Failed to close the service account token file", "serviceAccount", saName, "namespace", namespace, "path", tokenFilePath)
}

if err := os.Remove(tokenFilePath); err != nil {
log.Error(err, "Failed to clean up the service account token file", "serviceAccount", saName, "namespace", namespace, "path", tokenFilePath)
}
}()
Expand All @@ -240,6 +245,8 @@ func (r *ReplicatedPolicyReconciler) getToken(ctx context.Context, namespace str
return
}

log.V(1).Info("Refreshing the token", "serviceAccount", saName, "namespace", namespace)

returnedTokenReq, err := r.HubClient.CoreV1().ServiceAccounts(namespace).CreateToken(
ctx, saName, req, metav1.CreateOptions{},
)
Expand All @@ -251,6 +258,17 @@ func (r *ReplicatedPolicyReconciler) getToken(ctx context.Context, namespace str
continue
}

truncateErr := tokenFile.Truncate(0)
_, seekErr := tokenFile.Seek(0, 0)

if truncateErr != nil || seekErr != nil {
log.Error(errors.Join(truncateErr, seekErr), "Failed to overwrite the token file. Will retry in 5 seconds.")

time.Sleep(5 * time.Second)

continue
}

_, err = tokenFile.Write([]byte(returnedTokenReq.Status.Token))
if err != nil {
log.Error(err, "Failed to write the token. Will retry in 5 seconds.", "serviceAccount", saName, "namespace", namespace, "path", tokenFilePath)
Expand All @@ -271,8 +289,25 @@ func (r *ReplicatedPolicyReconciler) getToken(ctx context.Context, namespace str
}
```

#### Permission Changes to the Service Account

When permissions change after a watch has started, authorization is not reassessed until there is a new request, even
when the token expires. The [kubernetes-dependency-watches](https://github.com/stolostron/kubernetes-dependency-watches)
library leverages the client-go [RetryWatcher](https://pkg.go.dev/k8s.io/client-go/tools/watch#RetryWatcher) and it does
not currently return an error if the user does not have `watch` permissions. For when the watch is first started,
[PR #24](https://github.com/stolostron/kubernetes-dependency-watches/pull/24) works around this and will return an error
to the user indicating the user does not have `watch` permissions. If the `RetryWatcher` gets disconnected from the
Kubernetes API server and retries, it won't error out if the user no long has `watch` permissions. This is being
addressed in [kubernetes PR #126038](https://github.com/kubernetes/kubernetes/pull/126038). This will be a gap until the
upstream contribution is accepted and released. A restart of the Governance Policy Propagator would cause the error to
surface up to the user.

### Risks and Mitigation

- Existing policies would be able to leverage service accounts in the namespace and could take advantage of the
permissions associated with the service account. The best practice of not mixing policies and workloads in the same
namespace mitigates this.

### Open Questions [optional]

None yet.
Expand All @@ -298,6 +333,8 @@ N/A
## Drawbacks

- Complex implementation
- The user must manage an additional service account with makes the user experience more complex in some sense but
avoids the need for separate policies that copy content to the policy namespace.

## Alternatives

Expand Down

0 comments on commit fcc5fa0

Please sign in to comment.