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

Remove index audit output type #37707

Merged
merged 14 commits into from
Jan 24, 2019

Conversation

albertzaharovits
Copy link
Contributor

Supersedes #37301

This PR removes the Index Audit Output type, following its deprecation in 6.7 by #37671 . It also adds the migration notice (settings notice).

Closes #29881

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-security

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

nice stats. I left some comments

These settings enabled and configured the audit index output type. This output
type has been removed because it was unreliable in certain scenarios and this
could have lead to dropping audit events while the operations on the system
were allowed to continue as usual. This is a terrible failure state for an
Copy link
Member

Choose a reason for hiding this comment

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

Can we leave out This is a terrible failure state for an audit system which brought about its demise. ?

information, but it uses the older (pre-6.5.0) formatting style.
If the backwards compatible format is not required, it should be disabled.
To do that, change its logger level to `off` in the `log4j2.properties` file.
For backwards compatibility reasons, a `<clustername>_access.log` file is also
Copy link
Member

Choose a reason for hiding this comment

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

These are master docs, so I think this doesn't apply anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right 🙂 At first, this made me believe I still had a PR to do to remove the bwc audit format, that's how much I trust our docs!

x-pack/docs/en/security/auditing/overview.asciidoc Outdated Show resolved Hide resolved
x-pack/docs/en/security/auditing/overview.asciidoc Outdated Show resolved Hide resolved
jaymode and others added 4 commits January 22, 2019 17:44
Co-Authored-By: albertzaharovits <albert.zaharovits@gmail.com>
…ecurity/Security.java

Co-Authored-By: albertzaharovits <albert.zaharovits@gmail.com>
@albertzaharovits
Copy link
Contributor Author

albertzaharovits commented Jan 22, 2019

nice stats. I left some comments

++ 😀

Thanks @jaymode ! I've addressed your comments.

Copy link
Member

@jaymode jaymode left a comment

Choose a reason for hiding this comment

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

I left two minor comments. Otherwise LGTM

@albertzaharovits
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/1

@albertzaharovits
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/2

@albertzaharovits
Copy link
Contributor Author

18:58:42 FAILURE: Build failed with an exception.
18:58:42
18:58:42 * Where:
18:58:42 Build file '/tmp/junit14158968466601980319/build.gradle' line: 48
18:58:42
18:58:42 * What went wrong:
18:58:42 A problem occurred evaluating root project 'junit14158968466601980319'.
18:58:42 > Could not get unknown property 'unitTest' for root project 'junit14158968466601980319' of type org.gradle.api.Project.

@elasticmachine run elasticsearch-ci/1

@albertzaharovits
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/packaging-sample

Copy link
Contributor

@tvernum tvernum left a comment

Choose a reason for hiding this comment

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

LGTM.
We could probably do a followup to simplify/remove AuditTrail etc, but that's not needed here.

@albertzaharovits albertzaharovits merged commit b6936e3 into elastic:master Jan 24, 2019
@albertzaharovits albertzaharovits deleted the remove_audit_index branch January 24, 2019 10:44
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 24, 2019
* elastic/master:
  Optimize warning header de-duplication (elastic#37725)
  Bubble exceptions up in ClusterApplierService (elastic#37729)
  SQL: Improve handling of invalid args for PERCENTILE/PERCENTILE_RANK (elastic#37803)
  Remove unused ThreadBarrier class (elastic#37666)
  Add built-in user and role for code plugin (elastic#37030)
  Consolidate testclusters tests into a single project (elastic#37362)
  Fix docs for MappingUpdatedAction
  SQL: Introduce SQL DATE data type (elastic#37693)
  disabling bwc test while backporting elastic#37639
  Mute ClusterDisruptionIT testAckedIndexing
  Set acking timeout to 0 on dynamic mapping update (elastic#31140)
  Remove index audit output type (elastic#37707)
  Mute FollowerFailOverIT testReadRequestsReturnsLatestMappingVersion
  [ML] Increase close job timeout and lower the max number (elastic#37770)
  Remove Custom Listeners from SnapshotsService (elastic#37629)
  Use m_m_nodes from Zen1 master for Zen2 bootstrap (elastic#37701)
  Fix index filtering in follow info api. (elastic#37752)
  Use project dependency instead of substitutions for distributions (elastic#37730)
  Update authenticate to allow unknown fields (elastic#37713)
  Deprecate HLRC EmptyResponse used by security (elastic#37540)
albertzaharovits added a commit that referenced this pull request Feb 27, 2023
This PR removes:
 * the CompositeAuditTrail which is designed to fan-out
auditing events to multiple logger implementation types.
This is not needed because since v7.0 there's only one
audit logger implementation, the logfile.
 * any traces of the index-based logger implementation,
namely the permission of the internal _xpack user to
read the audit log index.

Related: #37707
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants