fix: ensure oathkeeper controller deployment correctly account for optionally created resources. #669
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
cbrendanprice
force-pushed
the
fix/oathkeeper-controller-nonmanaged-access-rules
branch
from
28cbd1a
to
ed6146f
Compare
cbrendanprice
changed the title
cbrendanprice
changed the title
cbrendanprice
changed the title
cbrendanprice
changed the title
cbrendanprice
changed the title
cbrendanprice
force-pushed
the
fix/oathkeeper-controller-nonmanaged-access-rules
branch
from
ed6146f
to
389c1f8
Compare
cbrendanprice
changed the title
cbrendanprice
force-pushed
the
fix/oathkeeper-controller-nonmanaged-access-rules
branch
from
389c1f8
to
b83e148
Compare
ensures that the controller correctly accounts for optionally created configmap and secret resources corresponding to the following values: - `oathkeeper.managedAccessRules` - `secret.enabled`
cbrendanprice
force-pushed
the
fix/oathkeeper-controller-nonmanaged-access-rules
branch
from
b83e148
to
076e657
Compare
Demonsthere
reviewed
Mar 28, 2024
There was a problem hiding this comment.
Hi there :) I think this issue was mentioned in the past. I have updated the PR to satisfy kubescape, besides that i think we can merge as is.
The idea was that oathkeeper as a application requires some rules to be present for it to start properly. In an k8s environment we dont want the deployment to be restarted until some CR is created, therefore we created the empty file to satisfy the startup conditions.
Demonsthere
approved these changes
Mar 28, 2024
|
Hi there! I recently upgraded the chart and found that it is no longer possible to load a configmap with access rules due to this PR. Was this the intended behavior? If I think what is missing is to check that both |
|
After some thought I think the write solution might be to set this to only work if maester is enabled. |
edeustua
added a commit
to examol-corp/ory-k8s
that referenced
this pull request
May 17, 2024
Since PR ory#669, self-managed access rules are impossible to deploy. This commit fixes that by allowing the deployment controller to load the rules config map when maester is disabled.
edeustua
added a commit
to examol-corp/ory-k8s
that referenced
this pull request
May 17, 2024
Since PR ory#669, self-managed access rules are impossible to deploy. This commit fixes that by allowing the deployment controller to load the rules config map when maester is disabled.
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fix ensures that the oathkeeper controller deployment template correctly accounts for optional creation of configmap and secrets based on the provided values:
oathkeeper.secret.enabledsecret.enabledRelated Issue or Design Document
n/a (couldn't find an open/closed issue for the controller failing to initialize due to configmap missing for rules)
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact security@ory.sh) from the maintainers to push the changes.
Further comments
this pr assumed that it should be possible to deploy the maester as a controller whilst also allowing it to manage the access rules. if this is incorrect, im happy to adjust the logic to ensure that the configmap for access rules get created unles both
managedAccessRulesisfalseand the maester mode issidecar.also please let me know if i need to increment the chart's semver patch for fixes; wasn't clear in the checklist above.