-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
policy: Error out on missing secrets in policy computation #9897
Conversation
Currently missing secrets cause empty TLS contexts to be sent to Envoy, which will then NACK the policy. Error out earlier in the Cilium endpoint regeneration logic so that Envoy is not bothered with errorneous configuration. Currently this will still lead to endpoint being not-ready, but at least the endpoint log contains the failure message pinpointing the problem (e.g., "Error regenerating endpoint: unable to regenerate policy for '310': secrets \"swapi-server\" not found"). Signed-off-by: Jarno Rajahalme <jarno@covalent.io>
Release note label not set, please set the appropriate release note. |
2 similar comments
Release note label not set, please set the appropriate release note. |
Release note label not set, please set the appropriate release note. |
} | ||
case OriginatingTLS: | ||
if ca == "" { | ||
return nil, fmt.Errorf("Originating TLS context is missing CA certs.") |
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.
error strings should not be capitalized or end with punctuation or a newline
switch direction { | ||
case TerminatingTLS: | ||
if public == "" || private == "" { | ||
return nil, fmt.Errorf("Terminating TLS context is missing certs.") |
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.
error strings should not be capitalized or end with punctuation or a newline
type TLSDirection string | ||
|
||
const ( | ||
TerminatingTLS TLSDirection = "terminating" |
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.
exported const TerminatingTLS should have comment (or a comment on this block) or be unexported
@@ -390,19 +390,40 @@ func (l7 L7DataMap) addRulesForEndpoints(rules api.L7Rules, endpoints []CachedSe | |||
} | |||
} | |||
|
|||
func (l4 *L4Filter) getCerts(policyCtx PolicyContext, tls *api.TLSContext, direction string) *TLSContext { | |||
type TLSDirection string |
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.
exported type TLSDirection should have comment or be unexported
test-me-please |
Currently missing secrets cause empty TLS contexts to be sent to
Envoy, which will then NACK the policy. Error out earlier in the
Cilium endpoint regeneration logic so that Envoy is not bothered with
errorneous configuration.
Currently this will still lead to endpoint being not-ready, but at
least the endpoint log contains the failure message pinpointing the
problem (e.g., "Error regenerating endpoint: unable to regenerate
policy for '310': secrets "swapi-server" not found").
Signed-off-by: Jarno Rajahalme jarno@covalent.io
This change is![Reviewable](https://camo.githubusercontent.com/23b05f5fb48215c989e92cc44cf6512512d083132bd3daf689867c8d9d386888/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)