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

aws_instance: fix broken user_data() #507

Merged
merged 1 commit into from
Feb 6, 2024

Conversation

syuu1228
Copy link
Contributor

@syuu1228 syuu1228 commented Feb 6, 2024

user_data() is mistakenly unsupported IMDSv2, since it's not using __instance_metadata().

Fixed the code to use __instance_metadata().

Fixes #498

@syuu1228
Copy link
Contributor Author

syuu1228 commented Feb 6, 2024

@syuu1228 syuu1228 requested a review from yaronkaikov February 6, 2024 16:03
user_data() is mistakenly unsupported IMDSv2, since it's not using
__instance_metadata().

Fixed the code to use __instance_metadata().

Fixes scylladb#498
Copy link
Collaborator

@yaronkaikov yaronkaikov left a comment

Choose a reason for hiding this comment

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

LGTM

@yaronkaikov yaronkaikov merged commit 6e92623 into scylladb:next Feb 6, 2024
1 check passed
@syuu1228
Copy link
Contributor Author

syuu1228 commented Feb 8, 2024

@yaronkaikov The patch is already merged though, I have one thing don't understand.

When I testing this patch with IMDSv2 mode userdata, I used example code of the userdata from our README.md.
Witch is something like this:

scylla_yaml:
  cluster_name: test-cluster
  experimental: true
  seed_provider:
    - class_name: org.apache.cassandra.locator.SimpleSeedProvider
      parameters:
        - seeds: 10.0.219.209
post_configuration_script: "#! /bin/bash\nyum install cloud-init-cfn"
start_scylla_on_first_boot: true

(The script was for CentOS7 so I modified the command to apt-get, but anyway)

But I realized that it causes "binascii.Error: Incorrect padding" exception while loading post_configuration_script.
Because our script expected post_configuration_script is base64 encoded, but the example YAML code doesn't.

So it only accepts something like this:

scylla_yaml:
  cluster_name: test-cluster
  experimental: true
  seed_provider:
    - class_name: org.apache.cassandra.locator.SimpleSeedProvider
      parameters:
        - seeds: 10.0.219.209
post_configuration_script: IyEgL2Jpbi9iYXNoCnl1bSBpbnN0YWxsIGNsb3VkLWluaXQtY2Zu # base64 encoded
start_scylla_on_first_boot: true

Entire userdata will encode in base64 anyway (at least on EC2 I think), so post_configuration_script will encoded twice.
I'm not sure this is correct behavior since our example code does not have such thing, so I added try-except condition on my patch to both:
https://github.com/scylladb/scylla-machine-image/pull/507/files#diff-71a59e03175d0b8ae7712c9a6ddc7bc6daeb902291a81b2f40ef59dda4d344f9R118

But I actually found both SCT and testcase in scylla-machine-image expected post_configuration_script is encoded in base64.

Do you know which way is correct?
If base64 encoded is the correct way, I will send new patch to drop my changes.

@fruch
Copy link
Collaborator

fruch commented Feb 8, 2024

post_configuration_script should be base64 encoded
is always was since the days it was a command line argument to cassandra scripts

it might be a nice extension feature to support also plain text, since now we are using yaml/json, but the base64 should still be working.

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

Successfully merging this pull request may close these issues.

Cannot get EC2 user data after enabled IMDSv2
3 participants