Skip to content
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

✨ add Audit webhook configuration options to RootShard/Shard objects #32

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

xrstf
Copy link
Contributor

@xrstf xrstf commented Jan 29, 2025

Summary

This PR adds new options to configure an external audit webhook. We chose to not support the local-volume audit logging options of kcp because unless we move to a StatefulSet, we won't be able to provision a PVC for each kcp replica Pod, and we cannot use the same PVC in multiple pods either.

This PR effectively wraps all of the following CLI flags:

      --audit-webhook-batch-buffer-size int         The size of the buffer to store events before batching and writing. Only used in batch mode. (default 10000)
      --audit-webhook-batch-max-size int            The maximum size of a batch. Only used in batch mode. (default 400)
      --audit-webhook-batch-max-wait duration       The amount of time to wait before force writing the batch that hadn't reached the max size. Only used in batch mode. (default 30s)
      --audit-webhook-batch-throttle-burst int      Maximum number of requests sent at the same moment if ThrottleQPS was not utilized before. Only used in batch mode. (default 15)
      --audit-webhook-batch-throttle-enable         Whether batching throttling is enabled. Only used in batch mode. (default true)
      --audit-webhook-batch-throttle-qps float32    Maximum average number of batches per second. Only used in batch mode. (default 10)
      --audit-webhook-config-file string            Path to a kubeconfig formatted file that defines the audit webhook configuration.
      --audit-webhook-initial-backoff duration      The amount of time to wait before retrying the first failed request. (default 10s)
      --audit-webhook-mode string                   Strategy for sending audit events. Blocking indicates sending events should block server responses. Batch causes the backend to buffer and write events asynchronously. Known modes are batch,blocking,blocking-strict. (default "batch")
      --audit-webhook-truncate-enabled              Whether event and batch truncating is enabled.
      --audit-webhook-truncate-max-batch-size int   Maximum size of the batch sent to the underlying backend. Actual serialized size can be several hundreds of bytes greater. If a batch exceeds this limit, it is split into several batches of smaller size. (default 10485760)
      --audit-webhook-truncate-max-event-size int   Maximum size of the audit event sent to the underlying backend. If the size of an event is greater than this number, first request and response are removed, and if this doesn't reduce the size enough, event is discarded. (default 102400)
      --audit-webhook-version string                API group and version used for serializing audit events written to webhook. (default "audit.k8s.io/v1")

I chose to not replicate the default values into the operator (neither in the code, nor in the comments) to prevent them becoming stale and not matching the actual kcp defaults anymore.

Related issue(s)

Fixes #13

Release Notes

Allow to configure an external audit webhook for kcp shards.

@kcp-ci-bot kcp-ci-bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. dco-signoff: yes Indicates the PR's author has signed the DCO. labels Jan 29, 2025
@xrstf xrstf requested a review from embik January 29, 2025 12:34
@kcp-ci-bot kcp-ci-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jan 29, 2025
@xrstf xrstf force-pushed the auditlogging-config branch from 056a35b to b3aae38 Compare January 30, 2025 14:52
@@ -37,6 +37,65 @@ type CommonShardSpec struct {

// Replicas configures how many instances of this shard run in parallel. Defaults to 2 if not set.
Replicas *int32 `json:"replicas,omitempty"`

KcpConfiguration KcpConfigurationSpec `json:"kcpConfiguration,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I like this, everything in the shard spec is "kcp configuration", more or less. I know where this is coming from but I wouldn't make such a distinction between app config and orchestrator config (e.g. you could argue that the etcd key is also "kcp configuration"). I think we can have the audit key directly in the struct.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, changed.

@xrstf xrstf force-pushed the auditlogging-config branch from b3aae38 to a9115c3 Compare February 3, 2025 14:12
Copy link
Member

@embik embik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/approve

@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Feb 3, 2025
@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: a900e4c05221248e8bd7495ee47a7391aaa9ac64

@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: embik

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 3, 2025
@kcp-ci-bot kcp-ci-bot merged commit bd8a00d into kcp-dev:main Feb 3, 2025
11 checks passed
@xrstf xrstf deleted the auditlogging-config branch February 4, 2025 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature: audit logging configuration
3 participants