Add LocalSecretStore #1256
Merged
Add LocalSecretStore #1256
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
pleshakov
added
the
enhancement
pleshakov
force-pushed
the
localsecretstore
branch
from
9e2218f to
a8f00a6
Compare
Dean-Coakley
approved these changes
Dec 2, 2020
internal/k8s/controller.go
Outdated
| glog.Warningf("Error validating secret %v for Ingress %v: %v", secretName, ing.Name, err) | ||
| secret = nil | ||
| } | ||
| glog.Warningf("Error trying to get the secret %v for Ingress %v: %v", secretName, ing.Name, err) |
There was a problem hiding this comment.
Should we add some context about the namespace of at least one of the resources?
Suggested change
| glog.Warningf("Error trying to get the secret %v for Ingress %v: %v", secretName, ing.Name, err) | |
| glog.Warningf("Error trying to get the secret %v for Ingress %v/%v: %v", secretKey, ing.Namespace, ing.Name, err) |
internal/k8s/controller.go
Outdated
| @@ -2227,58 +2164,84 @@ func (lbc *LoadBalancerController) getPolicies(policies []conf_v1.PolicyReferenc | |||
| return result, errors | |||
| } | |||
|
|
|||
| func (lbc *LoadBalancerController) addJWTSecrets(policies []*conf_v1alpha1.Policy, jwtKeys map[string]*api_v1.Secret) error { | |||
| func (lbc *LoadBalancerController) addJWTSecrets(secrets map[string]*configs.SecretReference, policies []*conf_v1alpha1.Policy) error { | |||
There was a problem hiding this comment.
Should we call this var(and all similar references) secretRefs so the reader can assume it's type to be map[string]*configs.SecretReference instead of map[string]*api_v1.Secret without reading the func signature?
Suggested change
| func (lbc *LoadBalancerController) addJWTSecrets(secrets map[string]*configs.SecretReference, policies []*conf_v1alpha1.Policy) error { | |
| func (lbc *LoadBalancerController) addJWTSecrets(secretRefs map[string]*configs.SecretReference, policies []*conf_v1alpha1.Policy) error { |
vepatel
reviewed
Dec 2, 2020
| @@ -352,7 +352,7 @@ def test_jwt_policy_delete_secret( | |||
| assert f"Request ID:" in resp1.text | |||
| assert crd_info["status"]["state"] == "Warning" | |||
| assert ( | |||
| f'"{v_s_route_setup.route_m.namespace}/{secret}" which does not exist' | |||
| "references an invalid Secret: secret doesn't exist or of an unsupported type" | |||
There was a problem hiding this comment.
No more secret name in warning message?
There was a problem hiding this comment.
good point. I can improve those messages in the next PR that implements remaining warnings
pleshakov
force-pushed
the
localsecretstore
branch
from
a8f00a6 to
0f3b908
Compare
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.
Proposed changes
This PR introduces LocalSecretStore for storing secrets (TLS, CA, JWK).
LocalSecretStore validates secrets and updates the secret contents on the file system when a secret is updated.
The Ingress Controller code was simplified as it is no longer necessary to update the secret contents on the file system on every regeneration of Ingress (or VS) config.