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

Silence logs generated by aws-vault on the AWS shell plugin #297

Merged
merged 5 commits into from Jun 26, 2023

Conversation

arunsathiya
Copy link
Contributor

Overview

When any aws command is run, the shell plugin currently outputs some additional logs related to the config file reader/importer. With the changes proposed on this PR, we'd hide those logs for now, but a long-term effort would be to identify why those logs are appearing and not printing them in the first place.

I have used the same solution that @williamhpark outlined on the issue.

Type of change

  • Created a new plugin
  • Improved an existing plugin
  • Fixed a bug in an existing plugin
  • Improved contributor utilities or experience

Related Issue(s)

How To Test

  • Clone this branch
  • Run make aws/build
  • Set up AWS shell plugin with op plugin init aws
  • Run any AWS command, like aws s3 ls
  • Make sure that no additional logs appear before the aws s3 ls's output appears.

Here's how it looks for me with the proposed changes:

image

Changelog

Silence AWS config file import logs while running any aws commands

@arunsathiya arunsathiya added the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label Jun 16, 2023
@arunsathiya arunsathiya self-assigned this Jun 16, 2023
@AndyTitu
Copy link
Contributor

There is another place where you could see the logs, in the aws-vault importer. Let's handle that as well.

Copy link
Contributor

@AndyTitu AndyTitu left a comment

Choose a reason for hiding this comment

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

After some testing I've spotted 3 places where we need to add ExecuteSilently
Screenshot 2023-06-21 at 18 21 22

  1. The first log which appears in that screenshot comes from function configLoader.LoadFromProfile(profile). This means that silencing confighelpers.LoadConfigFromEnv is not enough. We need to silence our whole getAWSAuthConfigurationForProfile (run cmd + F in sts_provisioner.go with the function name to find where it's called) function.

  2. The second log comes from p.AssumeRoleProvider.Retrieve(ctx) (run cmd + F in sts_provisioner.go with p.AssumeRoleProvider.Retrieve(ctx) to find where it's called). We need to silence this as well using the generic function.

  3. Similar to 2 we have p.SessionTokenProvider.Retrieve(ctx) (run cmd + F in sts_provisioner.go with p.SessionTokenProvider.Retrieve(ctx) to find where it's called) which could output a log, so let's silence that as well with the generic silencing function

UPDATE: Instead of silencing 2 and 3 individually, let's silence at a higher-up level: tempCredentialsProvider.Retrieve(ctx) in Provision. This is because sometimes assume role provider uses session token provider and the way our silencing function works, in this case the session token provider will remove silencing for assume role provider when it's done. (tested this locally)

plugins/aws/utils.go Outdated Show resolved Hide resolved
plugins/aws/utils.go Outdated Show resolved Hide resolved
@arunsathiya
Copy link
Contributor Author

I have tested both the aws sts get-caller-identity --profile value and importer flows, and I can't see those logs on either anymore.

image image

The error in second image encountered error while importing credentials automatically occurs even when the aws-vault importer flow is not dismissed, plus it seems to be tracked already here:

I haven't taken a look at that issue in detail yet, but all the logs outlined on this PR are handled now.

Copy link
Contributor

@AndyTitu AndyTitu left a comment

Choose a reason for hiding this comment

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

Tested latest changes locally and no logs are shown.

Also code LGTM

Copy link
Member

@hculea hculea left a comment

Choose a reason for hiding this comment

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

Love the approach with generics!

Code looks good to me. (I did not functionally test, let me know if you'd like more validation there as well).

@arunsathiya arunsathiya changed the title Silence AWS config file import logs while running any aws commands Silence logs generated by aws-vault on the AWS shell plugin Jun 23, 2023
@AndyTitu AndyTitu merged commit 509b723 into main Jun 26, 2023
5 checks passed
@AndyTitu AndyTitu deleted the arun/aws-silence-logs branch June 26, 2023 08:57
@arunsathiya arunsathiya removed the waiting-on-reviewer signals that a certain PR is waiting for a review from a 1Password team member label Aug 15, 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.

Silence AWS plugin logs
3 participants