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

New Module: Manage Windows event logs (win_eventlog) #27827

Merged
merged 2 commits into from
Aug 15, 2017

Conversation

andrewsaraceni
Copy link
Contributor

SUMMARY

Currently, no module exists for managing Windows event logs, as discussed in #26825. This module allows for the addition, clearing and removal of event logs and their associated sources. Additional settings for logs and sources can also be specified and applied, including:

  • category_file
  • message_file
  • parameter_file
  • maximum_size
  • overflow_action
  • retention_days

Integration tests and check-mode support are both included.

ISSUE TYPE
  • New Module Pull Request
COMPONENT NAME

lib/ansible/modules/windows/win_eventlog.ps1

ANSIBLE VERSION
ansible 2.4.0
ADDITIONAL INFORMATION
- name: Add a new event log with two custom sources
  win_eventlog:
    name: MyNewLog
    sources:
      - NewLogSource1
      - NewLogSource2
    state: present

- name: Change the category and message resource files used for NewLogSource1
  win_eventlog:
    name: MyNewLog
    sources:
      - NewLogSource1
    category_file: C:\NewApp\CustomCategories.dll
    message_file: C:\NewApp\CustomMessages.dll
    state: present

- name: Change the maximum size and overflow action for MyNewLog
  win_eventlog:
    name: MyNewLog
    maximum_size: 16MB
    overflow_action: DoNotOverwrite
    state: present

- name: Clear event entries for MyNewLog
  win_eventlog:
    name: MyNewLog
    state: clear

- name: Remove NewLogSource2 from MyNewLog
  win_eventlog:
    name: MyNewLog
    sources:
      - NewLogSource2
    state: absent

- name: Remove MyNewLog and all remaining sources
  win_eventlog:
    name: MyNewLog
    state: absent

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 community_review In order to be merged, this PR must follow the community review workflow. module This issue/PR relates to a module. needs_triage Needs a first human triage before being processed. new_module This PR includes a new module. new_plugin This PR includes a new plugin. support:community This issue/PR relates to code supported by the Ansible community. windows Windows community labels Aug 7, 2017
@ansibot
Copy link
Contributor

ansibot commented Aug 7, 2017

@SamLiu79 @timothyvandenbrande @andrewsaraceni @angstwad @ar7z1 @blakfeld @brianlloyd @chrishoffman @daBONDi @elventear @if-meaton @joshludwig @petemounce @rndmh3ro @schwartzmx @smadam813

As a maintainer of a module in the same namespace this new module has been submitted to, your vote counts for shipits. Please review this module and add shipit if you would like to see it merged.

click here for bot help

Copy link
Contributor

@jborean93 jborean93 left a comment

Choose a reason for hiding this comment

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

I'm pretty happy with how this looks, the only bits of feedback I have which doesn't need to stop merging this in if others don't agree are

  • should this be split up into 2 modules, one to manage the log and another to manage the sources of the logs
  • The choices for overflow_action aren't in snake_case, it looks easier the way you have it with direct mappings to the cmdlet values but should this be more in line with Ansible's style?
    I'll let others see if they agree or disagree with anything I've said. Thanks for the awesome work.

@jborean93
Copy link
Contributor

shipit

@ansibot ansibot added shipit This PR is ready to be merged by Core and removed community_review In order to be merged, this PR must follow the community review workflow. labels Aug 7, 2017
@andrewsaraceni
Copy link
Contributor Author

Thanks @jborean93! I actually explored splitting them into a separate module for logs and sources initially, but there's some pretty tight coupling between the two, e.g. the fact that a log cannot be created without a source. You'd have to provide the *_file options on both modules to account for this (which would be redundant functionality), or not allow the default/initial source to use alternate resource files to avoid that.

Additionally, the fact that the default (i.e. same-named) source for the log cannot be modified after creation would've seemingly made a sources module less usable in certain cases, and an arguably more awkward workflow for dealing with CRUDing logs/sources overall, so I decided to stick with one considering how they're managed in PowerShell.

Happy to hear others' thoughts on this though.

@ryansb ryansb removed the needs_triage Needs a first human triage before being processed. label Aug 8, 2017
@jborean93
Copy link
Contributor

That sounds fair enough, sounds like it should be one module.

@bcoca bcoca merged commit b0db1a1 into ansible:devel Aug 15, 2017
@andrewsaraceni andrewsaraceni deleted the win_eventlog branch August 15, 2017 18:45
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 module This issue/PR relates to a module. new_module This PR includes a new module. new_plugin This PR includes a new plugin. shipit This PR is ready to be merged by Core support:community This issue/PR relates to code supported by the Ansible community. windows Windows community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants