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.

Sign up for GitHub

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

Jump to bottom

tolerate more mime types for cc part of user-data. #234

Closed
wants to merge 11 commits into from
Closed

tolerate more mime types for cc part of user-data. #234

Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b2f4dd3
tolerate more mime types for cc part of user-data.
garloff Mar 5, 2020
1f476de
Avoid violating pycodestyle violation.
garloff Mar 5, 2020
556eb92
Merge remote-tracking branch 'upstream/master' into tolerate-more-cco…
garloff Mar 9, 2020
6687b96
Merge branch 'master' into tolerate-more-cconfig-mimetypes
garloff Mar 11, 2020
e786818
Merge branch 'master' into tolerate-more-cconfig-mimetypes
garloff Mar 25, 2020
a2bb184
Merge branch 'master' into tolerate-more-cconfig-mimetypes
OddBloke Mar 26, 2020
3640328
Merge branch 'master' into tolerate-more-cconfig-mimetypes
garloff Mar 26, 2020
70ddf96
Revert "Avoid violating pycodestyle violation."
garloff Mar 26, 2020
7168657
Revert "tolerate more mime types for cc part of user-data."
garloff Mar 26, 2020
35a6030
Fix cloud-init ignoring some mimetypes in user-data.
garloff Mar 27, 2020
4960778
Merge branch 'master' into tolerate-more-cconfig-mimetypes
garloff Mar 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion cloudinit/user_data.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@
CONTENT_TYPE = 'Content-Type'

# Various special content types that cause special actions
TYPE_NEEDED = ["text/plain", "text/x-not-multipart"]
TYPE_NEEDED = ["text/plain", "text/x-not-multipart",
"text/x-shellscript", "text/x-yaml"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I appreciate the goal here, but I don't really like the implementation.
I also accept that the reason this is not clear is my code. But please don't restrict me from making good decisions now because I made bad decisions in the past.

The strings you've added (text/x-shellscript) very much do not fit in 'TYPE_NEEDED'.
They're more 'type already supplied'.

I think more what we want to do is something like below. Ie, we already have
a list of types that we accept, lets just accept all of them. This would need
testing, and unfortunately (/me shakes fist at 10-years-ago-self) there are not
good unit tests on any of this code.

--- a/cloudinit/user_data.py
+++ b/cloudinit/user_data.py
@@ -117,7 +117,7 @@ class UserDataProcessor(object):
                 ctype_orig = UNDEF_TYPE
             if ctype_orig in TYPE_NEEDED:
                 ctype = find_ctype(payload)
-            if ctype is None:
+            if ctype in handlers.INCLUSION_TYPES_MAP.values() or ctype is None:
                 ctype = ctype_orig
 
             # In the case where the data was compressed, we want to make sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shouldn't we be safe by checking for the #cloud-config header also?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think that is necessary?
To me, if someone went to the trouble of creating a mime type of 'text/part-handler' then they probably meant that to be done. Or if they didn't... failure is fine.

I'm pretty sure that the rest of the code will trust the type. the prefixes are only used if there is a TYPE_NEEDED.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I will test whether your proposed change will work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does not work, I'm afraid:

[   18.302114] cloud-init[1122]: Cloud-init v. 19.4-33-gbb4131a2-0ubuntu1~18.04.1 running 'modules:config' at Fri, 13 Mar 2020 21:02:24 +0000. Up 17.80 seconds.
[[0;32m  OK  [0m] Started Apply the settings specified in cloud-config.
         Starting Execute cloud user/final scripts...
[   18.871485] cloud-init[1180]: Cloud-init v. 19.4-33-gbb4131a2-0ubuntu1~18.04.1 running 'modules:final' at Fri, 13 Mar 2020 21:02:25 +0000. Up 18.75 seconds.
[   18.873764] cloud-init[1180]: 2020-03-13 21:02:25,815 - util.py[WARNING]: Failed running /var/lib/cloud/instance/scripts/userdata [-]
[   18.878276] cloud-init[1180]: 2020-03-13 21:02:25,822 - cc_scripts_user.py[WARNING]: Failed to run module scripts-user (scripts in /var/lib/cloud/instance/scripts)
[   18.880637] cloud-init[1180]: 2020-03-13 21:02:25,825 - util.py[WARNING]: Running module scripts-user (<module 'cloudinit.config.cc_scripts_user' from '/usr/lib/python3/dist-packages/cloudinit/config/cc_scripts_user.py'>) failed

Should I attach the user-data maybe?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@garloff is the comment below the user-data (the multi-part mime)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@garloff is the comment below the user-data (the multi-part mime)?

Yes.

INCLUDE_TYPES = ['text/x-include-url', 'text/x-include-once-url']
ARCHIVE_TYPES = ["text/cloud-config-archive"]
UNDEF_TYPE = "text/plain"
Expand Down