Skip to content

Support for disabling legacy IMDS. #134

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

cernertrevor
Copy link

Hello. I am looking to offer an enhancement to packer-plugin-oracle to support the creation of instances with IMDSv1 support disabled (i.e. resolving #69 )

This is done by adding a new builder configuration option "instance_options" with a single attribute "are_legacy_imds_endpoints_disabled". This aligns with Oracle's CLI and Terraform's representation.

A few notes:

  • While I have worked with other languages before, I have not worked with Go; I would really appreciate any guidance around best practices as I was mostly trying to infer based on other OO languages.
  • I would also appreciate guidance on the tests. For config_test.go, I just added two tests to verify that the default is false, and when it is set to true that the configuration object has it set to true. I did not see any tests for driver_oci.go, so I'm not sure what should be done there.
  • I will make any and all changes recommended so long as the option is available to disable the IMDSv1 endpoints on Packer instances in OCI.
  • I ran some integration tests using my build of the plugin in my OCI tenancy, and the results were as expected:

IMDS Disabled using the following builder configuration:

"instance_options": {
  "are_legacy_imds_endpoints_disabled": true
},
imdsVersion2

And without the configuration, the current behavior:
imdsVersion1and2

Thank you for this opportunity.

Closes #69

@cernertrevor cernertrevor requested a review from a team as a code owner April 24, 2025 22:40
Copy link

hashicorp-cla-app bot commented Apr 24, 2025

CLA assistant check
All committers have signed the CLA.

@cernertrevor
Copy link
Author

Hi, @lbajolet-hashicorp. I apologize for tagging directly, but I saw you were referenced on another open PR. Would it be possible to get some eyes on mine? Thank you.

if d.cfg.InstanceOptions.AreLegacyImdsEndpointsDisabled != nil {
InstanceOptionsDetails := core.InstanceOptions{AreLegacyImdsEndpointsDisabled: d.cfg.InstanceOptions.AreLegacyImdsEndpointsDisabled}

instanceDetails.InstanceOptions = &InstanceOptionsDetails
Copy link
Contributor

Choose a reason for hiding this comment

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

Could be inlined maybe?

Copy link
Author

Choose a reason for hiding this comment

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

Sorry @neumayer , I was out last week, so didn't have a chance to get to this until now. I pulled the instance options inline. Thank you.

image

@@ -55,6 +55,10 @@ type ListImagesRequest struct {
Shape *string `mapstructure:"shape"`
}

type InstanceOptionsConfig struct {
AreLegacyImdsEndpointsDisabled *bool `mapstructure:"are_legacy_imds_endpoints_disabled" required:"false"`

Choose a reason for hiding this comment

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

i think this can be added directly to config no need of a new instance

@anshulsharma-hashicorp
Copy link

@cernertrevor please run make generate as some of the web docs are missing.

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.

Add option for disable legacy imds endpoints - security issue
3 participants