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

Add credentials endpoint option for awslogs driver #35055

Merged
merged 1 commit into from Oct 24, 2017

Conversation

@adnxn
Contributor

adnxn commented Oct 2, 2017

/cc @samuelkarp

- What I did
I added the awslogs driver option awslogs-credentials-endpoint to use relative URI paths to retrieve credentials from the aws sdk's default remote endpoint. The default remote endpoint for the sdk is at http://169.254.170.2. Otherwise, credentials from EC2 Instance Metadata will be used to send log events to cloudwatch logs. This new option would allow users to override that behavior, where they can specify a different relative uri path such as /v1/creds for the container's cloudwatch credentials.

- How I did it
Added credentialsEndpointKey to retrieve and consume the relative path and use that as the credentials provider endpoint.

- How to verify it
I added a test case to verifiy that the awslogs driver will retrieve credentials from a http endpoint. I've also added tests for the existing ways the awslogs driver can fetch credentials. It can be also be manually verified by starting a container with the extra --log-opt awslogs-credentials-endpoint=/v1/creds, assuming there is a http credential provider serving credentials from http://169.254.170.2/v1/creds

- Description for the changelog

awslogs driver can now fetch credentials from user defined relative URI endpoint.

@samuelkarp

LGTM after fixing the ineffassign issues.

info.Config["awslogs-credentials-endpoint"] = "/creds"
c, err := newAWSLogsClient(info)

This comment has been minimized.

@samuelkarp

samuelkarp Oct 2, 2017

Contributor

err is never read. You either want to use _ or actually check it. This also appears on lines 1128 and 1174.

daemon/logger/awslogs/cloudwatchlogs_test.go:1100:5⚠️ ineffectual assignment to err (ineffassign)

@samuelkarp

samuelkarp Oct 2, 2017

Contributor

err is never read. You either want to use _ or actually check it. This also appears on lines 1128 and 1174.

daemon/logger/awslogs/cloudwatchlogs_test.go:1100:5⚠️ ineffectual assignment to err (ineffassign)

This comment has been minimized.

@adnxn

adnxn Oct 3, 2017

Contributor

fixed.

@adnxn

adnxn Oct 3, 2017

Contributor

fixed.

Add credentials endpoint option for awslogs driver
Signed-off-by: Adnan Khan <adnkha@amazon.com>
@adnxn

This comment has been minimized.

Show comment
Hide comment
@adnxn

adnxn Oct 3, 2017

Contributor

z — Jenkins build Docker-PRs-s390x 6041 has failed. This is unrelated to changes made in the PR, specifically DockerSuite.TestUpdateKernelMemoryUninitialized in docker_cli_update_unix_test.go is failing.

Contributor

adnxn commented Oct 3, 2017

z — Jenkins build Docker-PRs-s390x 6041 has failed. This is unrelated to changes made in the PR, specifically DockerSuite.TestUpdateKernelMemoryUninitialized in docker_cli_update_unix_test.go is failing.

@yongtang

This comment has been minimized.

Show comment
Hide comment
@yongtang

yongtang Oct 3, 2017

Member

z failure is related to #34938

Member

yongtang commented Oct 3, 2017

z failure is related to #34938

@adnxn

This comment has been minimized.

Show comment
Hide comment
@adnxn

adnxn Oct 5, 2017

Contributor

cc @cpuguy83 PTAL

Contributor

adnxn commented Oct 5, 2017

cc @cpuguy83 PTAL

@yongtang

LGTM

@crosbymichael

This comment has been minimized.

Show comment
Hide comment
@crosbymichael

crosbymichael Oct 24, 2017

Contributor

LGTM

Contributor

crosbymichael commented Oct 24, 2017

LGTM

@crosbymichael crosbymichael merged commit 158c072 into moby:master Oct 24, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 37189 has succeeded
Details
janky Jenkins build Docker-PRs 45866 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 6248 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 17427 has succeeded
Details
z Jenkins build Docker-PRs-s390x 6084 has succeeded
Details
@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Oct 25, 2017

Member

@adnxn Thanks! Can you open a documentation pull-request against the vnext-engine branch in the docs repository? https://github.com/docker/docker.github.io/blob/vnext-engine/engine/admin/logging/awslogs.md

Member

thaJeztah commented Oct 25, 2017

@adnxn Thanks! Can you open a documentation pull-request against the vnext-engine branch in the docs repository? https://github.com/docker/docker.github.io/blob/vnext-engine/engine/admin/logging/awslogs.md

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment