-
Notifications
You must be signed in to change notification settings - Fork 4.6k
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
Implementing audit dynamic configuration (#7392) #7424
Implementing audit dynamic configuration (#7392) #7424
Conversation
Hi @mmerrill3. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/ok-to-test |
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.
Thank you! @mmerrill3
Actually I have been thinking to add this feature to our clusters as well. We have already falco running without k8s audit things.
If you can fix that small style thing, its good to go. Also please fix bazel things, one row missing there
nodeup/pkg/model/kube_apiserver.go
Outdated
@@ -347,6 +347,13 @@ func (b *KubeAPIServerBuilder) buildPod() (*v1.Pod, error) { | |||
} | |||
} | |||
|
|||
//remove elements from the spec that are not enabled yet | |||
if b.Cluster.Spec.KubeAPIServer.AuditDynamicConfiguration != nil { |
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.
if b.Cluster.Spec.KubeAPIServer.AuditDynamicConfiguration != nil && !b.IsKubernetesGTE("1.13") {
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.
ok, I'll push an update after this and rebasing with the upstream master again
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.
I updated bazel for the test runtime environment and this style update.
d343c0e
to
6040788
Compare
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.
/lgtm
/assign @justinsb
6040788
to
fde848c
Compare
rebased with the master branch to get the go mod fix |
@zetaab I had to rebase to pick up gomod changes. Could you kindly review again? |
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.
/lgtm
fde848c
to
617d965
Compare
I had to rebase again due to api updates in the master around service account parameters. This latest push is a rebase with those changes. |
617d965
to
957aabd
Compare
b78e391
to
344cc95
Compare
@justinsb I'll rebase again. Can this be PR be kept in mind next time the API structure for kops is updated again? |
344cc95
to
81a6b92
Compare
Signed-off-by: mmerrill3 <michael.merrill@vonage.com>
81a6b92
to
5cf94c8
Compare
@zetaab does it look good to you? |
/lgtm |
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.
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mmerrill3, zetaab 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 |
@mmerrill3 could you please cherry-pick this to release-1.17? |
Signed-off-by: mmerrill3 michael.merrill@vonage.com
This addresses issue #7392 to allow for dynamic audit configuration, so tools like falco can easily be integrated with kops to ingest audit logs through audit sinks.