Skip to content
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

Ensure chains in authservice don't get reordered and cause checksum changes #944

Closed
brianrexrode opened this issue Oct 21, 2024 · 3 comments · Fixed by #969
Closed

Ensure chains in authservice don't get reordered and cause checksum changes #944

brianrexrode opened this issue Oct 21, 2024 · 3 comments · Fixed by #969
Assignees
Labels
bug Something isn't working
Milestone

Comments

@brianrexrode
Copy link

Environment

Device and OS: Linux x86_64
App version: v0.28.0
Kubernetes distro being used: 1.30.1
Other:

Steps to reproduce

  1. Deploy 2 or more missions app that use authservice (via a uds-package for keycloak client and authservice chain creation)
  2. Restart the pepr-watcher pod

Expected result

pepr-watcher only updates the authservice-uds secret if something has changed and restart authservice pods accordingly

Actual Result

pepr-watcher may or may not reorder the chains in the authservice-uds secret regardless if any changes were made which causes the watcher to restart the authservice pod(s) for no reason.

This results in any active users being required to generate a new session token.

Visual Proof (screenshots, videos, text, etc)

slack thread here

Severity/Priority

Additional Context

Add any other context or screenshots about the technical debt here.

@brianrexrode brianrexrode added the possible-bug Something may not be working label Oct 21, 2024
@mjnagel mjnagel added bug Something isn't working and removed possible-bug Something may not be working labels Oct 21, 2024
@rjferguson21
Copy link
Contributor

I believe an easy fix would be to deterministically sort the chains of the authservice config so the checksum was consistent even if the Pepr watcher pod restarted.

See this function -

export function buildConfig(config: AuthserviceConfig, event: AuthServiceEvent) {
let chains: Chain[];
if (event.action == Action.Add) {
// Add the new chain to the existing authservice config
chains = config.chains.filter(chain => chain.name !== event.name);
chains = chains.concat(buildChain(event));
} else if (event.action == Action.Remove) {
// Search in the existing chains for the chain to remove by name
chains = config.chains.filter(chain => chain.name !== event.name);
} else {
throw new Error(`Unhandled Action: ${event.action satisfies never}`);
}
// Add the new chains to the existing authservice config
return { ...config, chains } as AuthserviceConfig;
}

I frequently use this one liner to take a quick peak at which chains are currently stored in the secret if its helpful -

kubectl get secrets -n authservice authservice-uds -o json | jq -r '.data["config.json"]' | base64 --decode | jq '.chains[].name

@catsby
Copy link
Contributor

catsby commented Oct 28, 2024

I opened #969 which should hopefully address this going forward by sorting the chains when they're added.

@catsby
Copy link
Contributor

catsby commented Oct 30, 2024

Hey @brianrexrode - we recently merged #969 to consistently sort the chains, which should address this issue going forward. Note that updating to the version that eventually contains this fix will still cause the issue as the current sorting of the chains would then be re-done, but after that there shouldn't be an issue. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants