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

Move AuditLogger interface to ecs-agent module #3653

Merged
merged 5 commits into from
Apr 26, 2023

Conversation

amogh09
Copy link
Contributor

@amogh09 amogh09 commented Apr 25, 2023

Summary

Moving AuditLogger interface to the new ecs-agent module. The interface will be used in future functionality to be added to ecs-agent module.

Implementation details

  • Move AuditLogger interface and the corresponding LogRequest struct to ecs-agent module.
  • Update the mockgen configuration in agent module to not create mock implementation for AuditLogger interface.
  • Add mockgen configuration to ecs-agent module to create mock implementation for AuditLogger interface in ecs-agent module.
  • Downgrade the version of testify used by ecs-agent to match the version used by agent module.
  • Make agent depend on ecs-agent and update the usage of AuditLogger and LogRequest to import them from ecs-agent module instead.

Testing

Built Agent from source and performed the following tests that both trigger audit logging.

  1. Sent requests to credentials endpoint and checked that logs are emitted to /var/log/ecs/audit.log file.
  2. Exceeded request rate limit of TMDS and checked that requests rejected due to rate limits are logged to /var/log/ecs/audit.log file.

New tests cover the changes: no

Description for the changelog

Move AuditLogger interface to ecs-agent module.

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@amogh09 amogh09 marked this pull request as ready for review April 25, 2023 18:03
@amogh09 amogh09 requested a review from a team as a code owner April 25, 2023 18:03
@amogh09 amogh09 changed the title Move AuditLogger interface to ecs-agent Move AuditLogger interface to ecs-agent module Apr 25, 2023
ohsoo
ohsoo previously approved these changes Apr 25, 2023
ohsoo
ohsoo previously approved these changes Apr 25, 2023
fierlion
fierlion previously approved these changes Apr 25, 2023
Copy link
Member

@fierlion fierlion left a comment

Choose a reason for hiding this comment

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

one nit -- but not blocking on it. If you can take a look in a future update, please do

agent/go.mod Outdated
@@ -3,6 +3,7 @@ module github.com/aws/amazon-ecs-agent/agent
go 1.19

require (
github.com/aws/amazon-ecs-agent/ecs-agent v0.0.0-00010101000000-000000000000
Copy link
Member

Choose a reason for hiding this comment

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

nit: was this go.mod auto-generated via go mod (I assume it was)?
can this just be v0.0.0 since we're replacing it anyway?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah this was auto-generated. I will simplify this.

@amogh09 amogh09 dismissed stale reviews from fierlion and ohsoo via 5677630 April 25, 2023 23:46
Copy link
Member

@fierlion fierlion left a comment

Choose a reason for hiding this comment

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

perfect in every way

@amogh09 amogh09 merged commit 9a8c796 into aws:dev Apr 26, 2023
39 checks passed
@amogh09 amogh09 deleted the move-audit-logger branch April 26, 2023 16:25
@Realmonia Realmonia mentioned this pull request May 9, 2023
Realmonia pushed a commit to Realmonia/amazon-ecs-agent that referenced this pull request May 16, 2023
Realmonia pushed a commit to Realmonia/amazon-ecs-agent that referenced this pull request May 21, 2023
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.

None yet

5 participants