Skip to content

feat: Add AWS instance identity authentication#570

Merged
kylecarbs merged 5 commits into
mainfrom
awsauth
Mar 28, 2022
Merged

feat: Add AWS instance identity authentication#570
kylecarbs merged 5 commits into
mainfrom
awsauth

Conversation

@kylecarbs
Copy link
Copy Markdown
Member

This allows zero-trust authentication for all AWS instances.

Prior to this, AWS instances could be used by passing CODER_TOKEN
as an environment variable to the startup script. AWS explicitly
states that secrets should not be passed in startup scripts because
it's user-readable.

@kylecarbs kylecarbs requested review from Emyrk and coadler March 26, 2022 01:56
@kylecarbs kylecarbs self-assigned this Mar 26, 2022
This allows zero-trust authentication for all AWS instances.

Prior to this, AWS instances could be used by passing `CODER_TOKEN`
as an environment variable to the startup script. AWS explicitly
states that secrets should not be passed in startup scripts because
it's user-readable.
@kylecarbs
Copy link
Copy Markdown
Member Author

@bpmct once this lands, we can remove CODER_TOKEN from examples in favor of the instance_id approach. Much more secure!

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 26, 2022

Codecov Report

Merging #570 (eec8e29) into main (be8389f) will increase coverage by 0.30%.
The diff coverage is 70.19%.

@@            Coverage Diff             @@
##             main     #570      +/-   ##
==========================================
+ Coverage   63.43%   63.73%   +0.30%     
==========================================
  Files         195      196       +1     
  Lines       11321    11514     +193     
  Branches       85       85              
==========================================
+ Hits         7181     7338     +157     
- Misses       3380     3399      +19     
- Partials      760      777      +17     
Flag Coverage Δ
unittest-go- 62.91% <70.19%> (+0.28%) ⬆️
unittest-go-macos-latest 58.47% <70.19%> (+0.47%) ⬆️
unittest-go-ubuntu-latest 61.42% <70.19%> (+0.43%) ⬆️
unittest-go-windows-2022 57.59% <70.19%> (+0.21%) ⬆️
unittest-js 63.32% <ø> (ø)
Impacted Files Coverage Δ
codersdk/workspaceresourceauth.go 47.50% <41.07%> (-15.00%) ⬇️
coderd/workspaceresourceauth.go 37.73% <44.44%> (+0.37%) ⬆️
coderd/awsidentity/awsidentity.go 67.44% <67.44%> (ø)
cli/workspaceagent.go 77.77% <90.62%> (+11.11%) ⬆️
coderd/coderdtest/coderdtest.go 98.63% <96.49%> (-0.53%) ⬇️
cli/cliui/prompt.go 78.26% <100.00%> (ø)
coderd/coderd.go 96.45% <100.00%> (+0.02%) ⬆️
provisioner/echo/serve.go 54.40% <0.00%> (-2.40%) ⬇️
coderd/workspaceresources.go 59.72% <0.00%> (-1.81%) ⬇️
provisionerd/provisionerd.go 80.02% <0.00%> (-0.15%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be8389f...eec8e29. Read the comment docs.

Comment on lines +18 to +30
const (
Other Region = "other"
HongKong Region = "hongkong"
Bahrain Region = "bahrain"
CapeTown Region = "capetown"
Milan Region = "milan"
China Region = "china"
GovCloud Region = "govcloud"
)

var (
All = []Region{Other, HongKong, Bahrain, CapeTown, Milan, China, GovCloud}
)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

These variable names should probably be prefixed with Region.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It felt unnecessarily verbose due to the small scope of the package.

Comment thread coderd/awsidentity/awsidentity.go Outdated
Comment thread coderd/coderdtest/coderdtest.go Outdated
Comment thread codersdk/workspaceresourceauth.go
Copy link
Copy Markdown
Member

@Emyrk Emyrk left a comment

Choose a reason for hiding this comment

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

I posted some nits. Nothing that will block a merge.

I am missing context on what this is actually used for though. It's only used for testing? We test in AWS and GCP?

Comment thread cli/workspaceagent_test.go
Comment thread coderd/coderdtest/coderdtest.go
Comment thread coderd/coderdtest/coderdtest.go
Comment thread coderd/coderdtest/coderdtest_test.go
Comment thread coderd/workspaceresourceauth_test.go
Comment thread codersdk/workspaceresourceauth.go
@kylecarbs kylecarbs enabled auto-merge (squash) March 28, 2022 19:16
@kylecarbs kylecarbs merged commit a502a5f into main Mar 28, 2022
@kylecarbs kylecarbs deleted the awsauth branch March 28, 2022 19:31
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