Promote AdvancedAuditing to GA #65862

loburm · 2018-07-05T13:18:54Z

What this PR does / why we need it:
Removes deprecated legacy code used for basic audit logging in favor of advanced audit logging.

Promote AdvancedAuditing to GA, replacing the previous (legacy) audit logging mechanisms.

loburm · 2018-07-05T13:22:25Z

/assign @timstclair @sttts @CaoShuFeng

Also I'm not completely sure about audit format of log file. Currently we have legacy and json. I suppose that legacy should be removed, together with this option, but I haven't managed to find any information related to this.

sttts · 2018-07-05T15:28:12Z

staging/src/k8s.io/apiserver/pkg/server/options/audit.go

@@ -266,11 +264,6 @@ func (o *AuditOptions) ApplyTo(c *server.Config) error {
 		return nil
 	}

-	// Apply legacy audit options if advanced audit is not enabled.
-	if !advancedAuditingEnabled() {


the whole gate should be removed as well

sttts · 2018-07-05T15:29:44Z

Also I'm not completely sure about audit format of log file. Currently we have legacy and json. I suppose that legacy should be removed, together with this option, but I haven't managed to find any information related to this.

Have we deprecated it properly in former releases?

sttts · 2018-07-06T07:43:06Z

This is the flag description:

--audit-log-format string                                 Format of saved audits. "legacy" indicates 1-line text format for each event. "json" indicates structured json format. Requires the 'AdvancedAuditing' feature gate. Known formats are legacy,json. (default "json")

I don't see that "legacy" was deprecated. So I don't think we can immediately remove it.

/hold

/cc @lavalamp @deads2k

loburm · 2018-07-06T07:52:49Z

@sttts that is correct. Also we haven't specified it in any release notes, which means that we need to support this flag for few more releases.

loburm · 2018-07-06T08:33:00Z

/test pull-kubernetes-verify
/test pull-kubernetes-e2e-kops-aws

loburm · 2018-07-06T09:26:36Z

/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-verify

loburm · 2018-07-09T12:22:20Z

/test pull-kubernetes-local-e2e
/test pull-kubernetes-e2e-kops-aws
/test pull-kubernetes-verify

loburm · 2018-07-10T06:31:40Z

@sttts is there any reason to hold this PR? I propose to deprecate --audit-log-format in the next PR. And then after two releases (1.14 I assume) we will be able to remove it.

@soltysh you may also be interested in reviewing it.

CaoShuFeng · 2018-07-10T06:34:40Z

Why --audit-log-format is bad?

loburm · 2018-07-10T07:02:36Z

@CaoShuFeng it's not bad, but currently it can handle two values: legacy and json. legacy is supposed to be removed with basic auditing, unless you see any reasons to leave it?

CaoShuFeng · 2018-07-10T07:06:44Z

I think --audit-log-format is part of advanced audit feature, not legacy audit feature.

CaoShuFeng · 2018-07-10T07:17:29Z

legacy is supposed to be removed with basic auditing

When --audit-log-format is set to legacy, we are still using advanced audit feature, I think.

sttts · 2018-07-10T08:08:39Z

is there any reason to hold this PR? I propose to deprecate --audit-log-format in the next PR. And then after two releases (1.14 I assume) we will be able to remove it.

@loburm not sure I can follow. It's hold because we cannot merge it before the deprecation time is over.

loburm · 2018-07-10T08:35:53Z

@sttts to which deprecation time are you referring here? Basic audit logging was deprecated in 1.8 and we can remove it now. --audit-log-format flag and both values haven't been deprecated and I'm not removing them in this PR.

loburm · 2018-08-23T19:00:17Z

@timstclair I have squashed and rebased.

One job is failing but according to dashboard it fails for all PRs.

tallclair · 2018-08-23T19:30:29Z

Thanks.

/lgtm

dims · 2018-08-27T14:03:44Z

/test pull-kubernetes-e2e-gke

@tallclair, folks, we need this for 1.12 right?

/milestone v1.12

loburm · 2018-08-29T14:07:27Z

/retest

loburm · 2018-08-29T15:15:01Z

/retest

CaoShuFeng · 2018-08-30T01:16:11Z

This needs one approver from cluster/gce/OWNERS

loburm · 2018-08-30T08:17:39Z

/assign @MaciekPytel

loburm · 2018-08-30T08:17:44Z

/retest

MaciekPytel · 2018-08-30T13:14:02Z

/approve

k8s-ci-robot · 2018-08-30T13:14:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: loburm, MaciekPytel, sttts, tallclair

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:

~~cluster/gce/OWNERS~~ [MaciekPytel]
~~hack/OWNERS~~ [sttts]
~~pkg/features/OWNERS~~ [tallclair]
~~pkg/kubeapiserver/OWNERS~~ [sttts]
~~staging/src/k8s.io/apiserver/OWNERS~~ [sttts]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

k8s-github-robot · 2018-08-30T13:15:56Z

/test all [submit-queue is verifying that this PR is safe to merge]

k8s-github-robot · 2018-08-30T14:31:22Z

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions here: https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md.

xref: kubernetes/kubernetes#65862 `AdvancedAuditing` feature is GA in 1.12. This PR adjusts the related docs.

kubernetes/kubernetes#65862

sttts · 2018-09-10T12:50:18Z

~~Did we forget to remove the legacy format constant?~~

Looks like it is still there:

kubernetes/staging/src/k8s.io/apiserver/pkg/audit/format.go

Line 63 in 7f23a74

    
           return fmt.Sprintf("%s AUDIT: id=%q stage=%q ip=%q method=%q user=%q groups=%q as=%q asgroups=%q namespace=%q uri=%q response=\"%s\"",

* audit 1.12 document * remove legacy audit feature kubernetes/kubernetes#65862 * update feature gate doc

loburm · 2018-09-12T10:57:47Z

Maybe I have misunderstood, but I thought that we don't want to remove this option because we haven't mentioned anywhere that it's going to be disabled (not documentation not in release notes). This option works with AdvancedAudit logging.

What I think we should do is to mark this option as deprecated in release notes and then remove after two cycles.

tallclair · 2018-09-19T23:04:49Z

Yeah, legacy format can still be requested with advanced auditing. It doesn't seem worth deprecating to me.

xref: kubernetes/kubernetes#65862 `AdvancedAuditing` feature is GA in 1.12. This PR adjusts the related docs.

soltysh · 2018-09-25T16:29:03Z

pkg/features/kube_features.go

@@ -406,7 +406,7 @@ var defaultKubernetesFeatureGates = map[utilfeature.Feature]utilfeature.FeatureS
 	// inherited features from generic apiserver, relisted here to get a conflict if it is changed
 	// unintentionally on either side:
 	genericfeatures.StreamingProxyRedirects: {Default: true, PreRelease: utilfeature.Beta},
-	genericfeatures.AdvancedAuditing:        {Default: true, PreRelease: utilfeature.Beta},
+	genericfeatures.AdvancedAuditing:        {Default: true, PreRelease: utilfeature.GA},


@loburm I think now based on kubernetes/website#10294 we should drop that feature gate.

Correct me if I'm wrong, but from my understanding now it's in GA and feature gate should be just deprecated (it's already no-op), and then after 6 months (2 releases) we can remove it.

Right, was it deprecated in that case? At least mention that in the release notes and update docs. Although I think usually we should print the deprecation warning, I think it's worth adding to 1.12.1.

Sorry I have missed this. Shall we deprecate it in 1.12.2 (not sure that policy allows us to do it in patch releases)? I would prefer to leave it for 1.13. WDYT?

* Update docs for fields allowed at root of CRD schema (#9973) * add plugin docs and examples (#10053) * docs update to promote TaintNodesByCondition to beta (#9626) * HPA Specificity Improvements (#8757) Updated the HPA docs to reference the `autoscaling/v2beta2` API version, and added documentation about the new fields. * adjust docs for pod ready++ (#10049) * Remove --cadvisor-port - has been deprecated since v1.10 (#10023) Change-Id: Id2a685473a243aef492a98ff450759f39e362557 * Add Documentation for Snapshot Feature (#9948) * Add documentation for snapshot feature * Update volume-snapshots.md * Add dry-run to api-concepts (#10033) * kubeadm-init: Update the offline support section (#10062) The update includes the following things (in mind with Kubernetes 1.12): - Remove the 1.8 image versions - Add the 1.10 image versions that were missing until now - Include a comment for the missing arch suffixes in 1.12 Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com> * Say bye to `DynamicProvisioningScheduling` (#10157) The mentioned feature gate is now collapsed into `VolumeScheduling`. xref: kubernetes/kubernetes#67432 * Update ResourceQuota per PriorityClass state for 1.12 (#10229) * TokenRequest and TokenRequestProjection now beta (#10161) xref: kubernetes/kubernetes#67349 * Change feature state for kms provider to beta. (#10230) KMS Provider will be graduating to beta in v1.12, reflecting this change on the website. * coredns default (#10200) * Promote ShareProcessNamespace to beta in docs (#9996) * Add CoreDNS details to DNS Debug docs (#10201) * add coredns details * address nits, add query logging section * Update docs with topology aware dynamic provisioning (#9939) * Document topology aware volume binding feature * update for readability * Update storage-classes.md * comma splice * don't abbreviate * HPA Algorithm Information Improvements (#9780) * Update HPA docs with more algorithm details The HPA docs pointed to an out-of-date document for information on the algorithm details, which users were finding confusing. This sticks a section on the algorithm in the HPA docs instead, documenting both general behavior and corner cases. * Add glossary info, HPA docs on quantities People often ask about the quantity notation when working with the metrics APIs, so this adds a glossary entry on quantities (since they're used elsewhere in the system), and a short explantation in the HPA walkthough. * Information about HPA readiness and stabilization This adds information about the new changes to HPA readiness and stabilization from kubernetes/enhancements#591, and other minor changes that landed in Kubernetes 1.12. * Update horizontal-pod-autoscale.md * Audit 1.12 doc (#9953) * audit 1.12 document * remove legacy audit feature kubernetes/kubernetes#65862 * update feature gate doc * MountPropagation is now GA (#10090) * RuntimeClass documentation (#10102) * RuntimeClass documentation * Update runtime-class.md * Add documentation for Scheduler performance tuning (#10048) * Add documentation for Scheduler performance tuning * Update scheduler-perf-tuning.md * TTL controller for cleaning up finished resources (#10064) * TTL controller for cleaning up finished resources * Address comments * Update ttlafterfinished.md * Bump quota configuration api version (#10217) * Incremental update from master (#10278) * fix invalid href of cloud controller manager (#10240) * fix invalid yaml format (#10238) * update storage-limits doc with Azure disk part (#10224) update storage-limits doc with Azure disk part fix comments * Update kubelet-config-file.md (#10222) Update link to KubeletConfiguration struct. * fix a trivial misspelling (#10244) * Fix cassandra-statefulset.yaml indent level (#10243) * Mention minimum etcd versions (#10208) Source: https://groups.google.com/d/msg/kubernetes-dev/jMPA4JzKiY4/HIx2ugvLBAAJ * fix 404 error (#10250) * Small verb tweak (#10190) Present participle, ftw. * Add AnchorJS logic for header links (#10155) * Add AnchorJS JavaScript * Remove existing inpage_heading logic * Remove underline from anchor tags * Use single icon and add touch visibility * Use paragraph link icon for AnchorJS * Update Sass to use code formatting in docsContent headers * Update header size coverage to H3-H6 * fix broken link in kubefed.md (#10254) * Update the version numbers for the X-Remote-Extra- and Impersonate-Extra- key fixes (#9827) The fix was cherry picked into 1.11.3, 1.10.7, and 1.9.11: kubernetes/kubernetes#67162 kubernetes/kubernetes#67163 kubernetes/kubernetes#67164 * fix typo (#10168) * fix typo * addressing comments. * Update setup-ha-etcd-with-kubeadm.md * fix typos (#10252) * fix description of contribute guide (#10253) * describe truncate feature about advanced audit (#10236) * describe truncate feature about advanced audit * Update audit.md * docs update to promote ScheduleDaemonSetPods to beta (#9923) * Dynamic volume limit updates for 1.12 (#10211) * add a placeholder commit * Update docs for csi volume limits * Update storage-limits.md * Add "MayRunAs" value among other GroupStrategies (#9888) * Add CoreDNS details to the customize DNS doc (#10228) * Add CoreDNS details to the customize DNS doc Rewrite the document to include more details about CoreDNS, since it's now the default from v1.12 * Address comments * Improve doc wording * Fix link * Update dns-custom-nameservers.md * Update dns-custom-nameservers.md * Fix secrets docs in 1.12 branch (#10056) * Fix secrets docs * Update secret.md * Revert CoreDNS Docs (#10319) * Revert "Add CoreDNS details to DNS Debug docs (#10201)" This reverts commit 462817a. * Revert "Add CoreDNS details to the customize DNS doc (#10228)" This reverts commit e7319ee. * Revert "coredns default (#10200)" This reverts commit 698e93b. * Add CRI installation instructions page Added cri-installation page with CRI installation instructions Referenced it from kubeadm-init and install-kubeadm pages. * kubeadm: update API types documentation for 1.12 (#10283) v1alpha2 -> v1alpha3 MasterConfiguration -> [new-api-types] * TokenRequest feature documentation (#10295) * AdvancedAuditing is now GA (#10156) xref: kubernetes/kubernetes#65862 `AdvancedAuditing` feature is GA in 1.12. This PR adjusts the related docs. * update runtime-class.md (#10332) * update runtime-class.md * Update runtime-class.md * Document cross-authorizer permissions for creating RBAC roles (#10015) * Document cross-authorizer permissions for creating RBAC roles * Update rbac.md * kubeadm: update authored content for 1.12 (reference docs and cluster creation) (#10348) * kubeadm: update authored content in reference docs for 1.12 * kubeadm: add time frame in create-cluster-kubeadm for 1.12 * add AllowedProcMountTypes and ProcMountType to docs (#9911) Signed-off-by: Jess Frazelle <acidburn@microsoft.com> * kubeadm: add new command line reference (#10306) Add: - placeholder files - include place holder files - include "renew" sub command - add missing tabs for "alpha phase kubelet" * Documenting SCTP support in Kubernetes (#10279) * Documenting SCTP support in Kubernetes Service, Endpoint, NetworkPolicy and Pod * Updates based on comments on the PR * kubectl expose update with SCTP support * Updated according to comments in the PR * Revert "kubectl expose update with SCTP support" This reverts commit 0d5a1e6. * TLS Bootstrap and Server Cert Rotation feature documentation (#10232) * TokenRequest feature documentation * line wrapping to make review not insane * update content for GA without major refactor * Update kubelet-tls-bootstrapping.md * Add clarifications for volume snapshots (#10296) * Update kubadm ha installation for 1.12 (#10264) * Update kubadm ha installation for 1.12 Signed-off-by: Chuck Ha <ha.chuck@gmail.com> * update stable version Signed-off-by: Chuck Ha <ha.chuck@gmail.com> * Update stacked control plane for v1.12 (#2) * use v1alpha3 Signed-off-by: Chuck Ha <ha.chuck@gmail.com> * more v1alpha3 (#4) * updates Signed-off-by: Chuck Ha <ha.chuck@gmail.com> * Document how to run in-tree cloud providers with kubeadm (#10357) Change-Id: Iab6b996a830503d74a6eb0c507c5f8ca7a39235b * kubeadm reference doc for release 1.12 (#10359) * Revert "Revert "Add CoreDNS details to DNS Debug docs (#10201)"" This reverts commit bb30f4d. * Revert "Revert "Add CoreDNS details to the customize DNS doc (#10228)"" This reverts commit bc23d45. * Revert "Revert "coredns default (#10200)"" This reverts commit 7f4350d. * add missing instruction for ha guide (#10374) Signed-off-by: Chuck Ha <ha.chuck@gmail.com> * kubeadm - Ha upgrade updates (#10340) * Update HA upgrade docs * Adds external etcd HA upgrade guide Signed-off-by: Chuck Ha <ha.chuck@gmail.com> * copyedit * more edits * add runasgroup in psp (#10076) * update KubeletPluginsWatcher feature gate (#10205) * generated 1.12 docs * Building Multi-arch images with Manifests (#10379) In 1.12, a variety of images used in a typical kubernetes installation have started to using manifests to better support environments with arm or ppc64le architectures. For example all images used with kubeadm by default have manifests, another would be all the tests in the conformance test suite. Here we capture the best practices for everyone to start using manifests in their own workflows. Change-Id: I5ba4c5fe55ffc9486a8251760f3352be4f2e1494 * Upgrade docs for v1.12 (#10344) * generated assets and docs * remove 1.7 * update 1.12 * update plugin documentation under docs>tasks>extend-kubectl (#10259) * update plugin documentation under docs>tasks>extend-kubectl * Update kubectl-plugins.md

k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jul 5, 2018

k8s-ci-robot requested review from bowei and deads2k July 5, 2018 13:19

k8s-ci-robot assigned CaoShuFeng, sttts and timstclair Jul 5, 2018

loburm changed the title ~~Remove basic audit logging code~~ [WIP] Remove basic audit logging code Jul 5, 2018

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 5, 2018

sttts reviewed Jul 5, 2018

View reviewed changes

loburm force-pushed the remove_basic_audit branch from 4501207 to d12130a Compare July 6, 2018 07:42

k8s-ci-robot requested a review from lavalamp July 6, 2018 07:43

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 6, 2018

loburm changed the title ~~[WIP] Remove basic audit logging code~~ Remove basic audit logging code Jul 6, 2018

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 6, 2018

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 23, 2018

tallclair changed the title ~~Remove basic audit logging code~~ Promote AdvancedAuditing to GA Aug 23, 2018

k8s-ci-robot added this to the v1.12 milestone Aug 27, 2018

k8s-ci-robot assigned MaciekPytel Aug 30, 2018

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 30, 2018

k8s-github-robot merged commit 58ead1c into kubernetes:master Aug 30, 2018

tengqm added a commit to tengqm/website that referenced this pull request Aug 31, 2018

AdvancedAuditing is now GA

b08fc50

xref: kubernetes/kubernetes#65862 `AdvancedAuditing` feature is GA in 1.12. This PR adjusts the related docs.

tengqm mentioned this pull request Aug 31, 2018

AdvancedAuditing is now GA kubernetes/website#10156

Merged

CaoShuFeng added a commit to CaoShuFeng/kubernetes.github.io that referenced this pull request Sep 3, 2018

remove legacy audit feature

fcf7734

kubernetes/kubernetes#65862

CaoShuFeng added a commit to CaoShuFeng/kubernetes.github.io that referenced this pull request Sep 6, 2018

remove legacy audit feature

1e23341

kubernetes/kubernetes#65862

k8s-ci-robot pushed a commit to kubernetes/website that referenced this pull request Sep 12, 2018

Audit 1.12 doc (#9953)

2673795

* audit 1.12 document * remove legacy audit feature kubernetes/kubernetes#65862 * update feature gate doc

k8s-ci-robot pushed a commit to kubernetes/website that referenced this pull request Sep 19, 2018

AdvancedAuditing is now GA (#10156)

0b786ff

xref: kubernetes/kubernetes#65862 `AdvancedAuditing` feature is GA in 1.12. This PR adjusts the related docs.

soltysh reviewed Sep 25, 2018

View reviewed changes

SataQiu mentioned this pull request Jan 18, 2023

Lock the AdvancedAuditing feature gate to true #115163

Merged

Promote AdvancedAuditing to GA #65862

Promote AdvancedAuditing to GA #65862

Conversation

loburm commented Jul 5, 2018 • edited by tallclair

loburm commented Jul 5, 2018

sttts Jul 5, 2018

Choose a reason for hiding this comment

loburm Jul 6, 2018

Choose a reason for hiding this comment

sttts commented Jul 5, 2018

sttts commented Jul 6, 2018

loburm commented Jul 6, 2018

loburm commented Jul 6, 2018

loburm commented Jul 6, 2018

loburm commented Jul 9, 2018

loburm commented Jul 10, 2018

CaoShuFeng commented Jul 10, 2018 • edited

loburm commented Jul 10, 2018

CaoShuFeng commented Jul 10, 2018

CaoShuFeng commented Jul 10, 2018

sttts commented Jul 10, 2018

loburm commented Jul 10, 2018

loburm commented Aug 23, 2018

tallclair commented Aug 23, 2018

dims commented Aug 27, 2018

loburm commented Aug 29, 2018

loburm commented Aug 29, 2018

CaoShuFeng commented Aug 30, 2018

loburm commented Aug 30, 2018

loburm commented Aug 30, 2018

MaciekPytel commented Aug 30, 2018

k8s-ci-robot commented Aug 30, 2018

k8s-github-robot commented Aug 30, 2018

k8s-github-robot commented Aug 30, 2018

sttts commented Sep 10, 2018 • edited

loburm commented Sep 12, 2018 • edited

tallclair commented Sep 19, 2018

soltysh Sep 25, 2018

Choose a reason for hiding this comment

loburm Sep 25, 2018

Choose a reason for hiding this comment

soltysh Sep 26, 2018

Choose a reason for hiding this comment

loburm Oct 10, 2018

Choose a reason for hiding this comment

loburm commented Jul 5, 2018 •

edited by tallclair

CaoShuFeng commented Jul 10, 2018 •

edited

sttts commented Sep 10, 2018 •

edited

loburm commented Sep 12, 2018 •

edited