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

docs: update docs to clarify that the IDs in user_ssh_key_ids are user IDs #322

Merged
merged 1 commit into from
May 18, 2023

Conversation

ctreatma
Copy link
Contributor

Our documentation incorrectly stated that the IDs in user_ssh_key_ids for an equinix_metal_device were the IDs of the SSH keys that will be supported on the device; in reality they are the IDs of the users whose SSH keys will be supported.

In addition to the documentation, our tests were incorrectly attempting to pass an SSH key ID as a user ID, resulting in test failures due to errors from the API.

@ctreatma ctreatma marked this pull request as draft May 16, 2023 15:14
@@ -905,7 +905,7 @@ resource "equinix_metal_device" "test" {
operating_system = local.os
billing_cycle = "hourly"
project_id = equinix_metal_project.test.id
user_ssh_key_ids = [equinix_metal_ssh_key.test.id]
user_ssh_key_ids = [equinix_metal_ssh_key.test.owner_id]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This fixes the 422 error from the API, but the test is currently checking that the device has 2 SSH keys, and that is an invalid expectation. It has every SSH key for the users specified in user_ssh_key_ids plus the 1 project SSH key specified in project_ssh_key_ids. I'm not really sure how to resolve this in a reliable way; can we instead check that the device SSH keys include the project and user SSH keys that were created as part of the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Test has been updated to confirm that the ssh_key_ids for the device include the IDs of the SSH keys that were created in the test: a3e2156#diff-b5bc3c33d7df2c76175fac51f0b7b4a5d4e2fd0268430d3b28091961885d775fR193-R204

@ctreatma ctreatma force-pushed the fix-user-ssh-key-docs branch from dfdee20 to a3e2156 Compare May 17, 2023 14:33
@ctreatma ctreatma marked this pull request as ready for review May 17, 2023 14:40
@ctreatma
Copy link
Contributor Author

Updated test passes: https://github.com/equinix/terraform-provider-equinix/actions/runs/5004498703/jobs/8967176160#step:5:3040

@codecov-commenter
Copy link

Codecov Report

Patch coverage has no change and project coverage change: -0.21 ⚠️

Comparison is base (d0a8ec6) 60.47% compared to head (a3e2156) 60.27%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #322      +/-   ##
==========================================
- Coverage   60.47%   60.27%   -0.21%     
==========================================
  Files          90       90              
  Lines       17940    17940              
==========================================
- Hits        10850    10813      -37     
- Misses       6793     6830      +37     
  Partials      297      297              
Impacted Files Coverage Δ
equinix/resource_metal_device.go 72.33% <ø> (-1.31%) ⬇️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ctreatma ctreatma force-pushed the fix-user-ssh-key-docs branch from a3e2156 to e02c75a Compare May 17, 2023 16:31
@ctreatma ctreatma merged commit 47034fb into master May 18, 2023
@displague
Copy link
Member

displague commented May 18, 2023

For reference:

@displague displague deleted the fix-user-ssh-key-docs branch May 18, 2023 14:33
@displague
Copy link
Member

Relates to #203

thogarty pushed a commit that referenced this pull request Sep 5, 2023
docs: update docs to clarify that the IDs in user_ssh_key_ids are user IDs
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