Authentication check: additional k8s Event check for Secret absence in source #2014
Conversation
google-cla
bot
added
the
cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: grac3gao The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
grac3gao-zz
changed the title
pkg/reconciler/intevents/pullsubscription/static/pullsubscription.go
Outdated
Show resolved
Hide resolved
pkg/utils/authcheck/list.go
Outdated
| @@ -65,3 +94,17 @@ func GetTerminationLogFromPodList(pl *corev1.PodList) string { | |||
| func isAuthMessage(message string) bool { | |||
| return strings.Contains(message, authMessage) | |||
| } | |||
|
|
|||
| // isWarningMessage checks if the message is for a specific secret's failure. | |||
| func isWarningMessage(message, namespace string, secret *corev1.SecretKeySelector) bool { | |||
There was a problem hiding this comment.
| func isWarningMessage(message, namespace string, secret *corev1.SecretKeySelector) bool { | |
| func isSecretMountFailureMessage(message, namespace string, secret *corev1.SecretKeySelector) bool { |
There was a problem hiding this comment.
This function checks message for two cases: key is absent, and secret is absent. The k8s Event Reason for those two case are different. For key is absent, the reason is just Failed, for secret is absent, the reason is FailedMount. So I think case key is absent is not strictly a secretMountFailure case. If you don't mind, I'll change it to isSecretFailureMessage.
There was a problem hiding this comment.
SGTM. I found the isWarningMessage name to be misleading, isSecretFailureMessage is much clearer.
pkg/reconciler/intevents/pullsubscription/static/pullsubscription_test.go
Outdated
Show resolved
Hide resolved
|
The following is the coverage report on the affected files.
|
There was a problem hiding this comment.
/lgtm
/hold
Holding in case @AlexandraRoatis has any additional comments.
|
/lgtm |
Fixes #
PRs to add the authentication check:
Proposed Changes
Release Note