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

feat: New spec and parser for"auditctl -l" #3496

Merged
merged 3 commits into from
Aug 18, 2022
Merged

feat: New spec and parser for"auditctl -l" #3496

merged 3 commits into from
Aug 18, 2022

Conversation

huali027
Copy link
Contributor

@huali027 huali027 commented Aug 15, 2022

Signed-off-by: Huanhuan Li [email protected]

All Pull Requests:

Check all that apply:

  • Have you followed the guidelines in our Contributing document, including the instructions about commit messages?
  • Is this PR to correct an issue?
  • Is this PR an enhancement?

This update is targeting to add a new spec and parser for "auditctl -l". Since there is an existing module auditctl_status, and the existing parser didn't follow the latest design. This PR also:

  • Mark the old "AuditctlStatus" parser as deprecated
  • Move the auditctl related parsers to auditctl module.

@huali027
Copy link
Contributor Author

@xiangce Please help to review.
Since there is already a parser for the command "auditctl status", I put them together. But the file name isn't general for "auditctl" commands. Currently, I'm afraid it will impact the existing parser path, I don't change it. Please suggest if I need to rename the file.

* Add a new module for all "auditctl" commands

Signed-off-by: Huanhuan Li <[email protected]>
deprecated(
AuditctlStatus,
"Please use the :class:`insights.parsers.auditctl.AuditdStatus` instead.",
"3.1.25"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@psachin after discussion with @xiangce , we agree to add a new module to include the parsers about "auditctl" commands, because the original file name is "auditctl_status.py", it's not good to add other "auditctl" commands. And I mark the original one as deprecated. Now the version is wrong, I'm not sure which version I should use, please help here, thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

3.1.25 is OK

Copy link
Contributor

Choose a reason for hiding this comment

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

Better introduce a global variable, e.g. "NEXT_MINOR_VER", to avoid such kind of confusion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently, the exact version list isn't done, so a few parsers are marked as "3.1.25". Sachin said it is a release which is one year later. After the version list comes out, maybe we'll mark different versions for the parser. Maybe in one release, there is only one parser deprecated. In this case, maybe a global variable is useless? Do we need to do it now for "3.1.25"?

Copy link
Contributor

@xiangce xiangce Aug 18, 2022

Choose a reason for hiding this comment

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

Sorry for the incomplete comment, that is for @psachin, but not a blocker for this PR, please go ahead and use the hard-coded 3.1.25 for this change.

Maybe in one release, there is only one parser deprecated. In this case, maybe a global variable is useless?

No matter how many component (even none) would be deprecated in the next minor release version, it would be better to use a global variable than the hard-coded versions, from the development and release perspective.

Copy link
Contributor

Choose a reason for hiding this comment

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

@xiangce By using custom version the developers can plan the feature deprecation accordingly. The developer might not want to deprecate the feature in NEXT_MINOR_VER. I have drafted a change in the docs with examples to help them in this process[1].

[1] #3500

@xiangce xiangce changed the title feat: New spec "auditctl -l" feat: New spec and parser for"auditctl -l" Aug 18, 2022
"""
def parse_content(self, content):
if not content:
raise ParseException("Input content is empty.")
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be a skip.

self.data[k.strip()] = int(v.strip())
except ValueError:
continue

Copy link
Contributor

Choose a reason for hiding this comment

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

Better skip it when get nothing parsered

* Raise "ParseException" when the line isn't in expected format instead of pass it,
  maybe we need to enhance the parser.
  update the module docstring.
  raise SkipException when there is no known status output

Signed-off-by: Huanhuan Li <[email protected]>
@xiangce xiangce merged commit 0bd77b4 into RedHatInsights:master Aug 18, 2022
xiangce pushed a commit that referenced this pull request Aug 18, 2022
* feat: New spec "auditctl -l"

* Add a new module for all "auditctl" commands

Signed-off-by: Huanhuan Li <[email protected]>

* Replace the parent class "LegacyItemAccess" to "dict"

* Raise "ParseException" when the line isn't in expected format instead of pass it,
  maybe we need to enhance the parser.
  update the module docstring.
  raise SkipException when there is no known status output

Signed-off-by: Huanhuan Li <[email protected]>

* Rename "AuditdStatus" to "AuditStatus"

Signed-off-by: Huanhuan Li <[email protected]>

Signed-off-by: Huanhuan Li <[email protected]>
(cherry picked from commit 0bd77b4)
xiangce pushed a commit that referenced this pull request Sep 6, 2024
* feat: New spec "auditctl -l"

* Add a new module for all "auditctl" commands

Signed-off-by: Huanhuan Li <[email protected]>

* Replace the parent class "LegacyItemAccess" to "dict"

* Raise "ParseException" when the line isn't in expected format instead of pass it,
  maybe we need to enhance the parser.
  update the module docstring.
  raise SkipException when there is no known status output

Signed-off-by: Huanhuan Li <[email protected]>

* Rename "AuditdStatus" to "AuditStatus"

Signed-off-by: Huanhuan Li <[email protected]>

Signed-off-by: Huanhuan Li <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants