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

Option to quote version in generated config to prevent misinterpretation #36

Open
mdegat01 opened this issue Nov 18, 2021 · 7 comments
Open
Labels
no-stale This issue or PR is exempted from the stable bot.

Comments

@mdegat01
Copy link

Problem/Motivation

On the edge repository of my addon I follow the same pattern as Home Assistant Community Add-ons and use the commit hash as the version number. The problem is that for the last update I pushed out the hash was 58899e0. This created an issue because that's actually a number if put unquoted in YAML which this action does when it generates the config.yaml file. Supervisor then misinterpreted this as version 58899.0 which caused me to be unable to update.

I realize this is probably quite rare but since it did happen I figured I would report it.

Expected behavior

Generated config.yaml contains the following:

version: '58899e0'

Actual behavior

Generated config.yaml contained this:

version: 58899e0

Here is the full config file for my add-on as generated by this action at the time of the issue.

Steps to reproduce

  1. Use repository updater to update an addon to version 58899e0
  2. Attempt to install/update that add-on in supervisor
    image
    image

Proposed changes

It would be good if the action had an input specifying whether to quote the value in the version field to prevent misinterpretation. Or perhaps if it quoted by default with an option to turn that off since I think most people likely use strings (semver, datever, commit hash, etc.)

@frenck
Copy link
Member

frenck commented Nov 18, 2021

Interesting, almost looks like a PyYAML bug, as the version is created like so:

self.latest_version = last_commit.sha[:7]

That is definitely a string...
Which is then written to the config and dumped to YAML:

config["version"] = self.current_version

...

yaml.dump(config, outfile, default_flow_style=False)

That would be something that gets lost in the dump handling. Not sure how to solve that even, as, well, it is already a string... I cannot make it "more" string than that 😟

@frenck
Copy link
Member

frenck commented Nov 18, 2021

What I can do to fix this, is making the repository updater always have a JSON file as a result, regardless of the add-on has a YAML configuration file or not.

Do you think that is reasonable?

@mdegat01
Copy link
Author

mdegat01 commented Nov 18, 2021

Yea that makes sense. Bit of a bummer to have to convert back to JSON but seems like the only quick fix.

I did some testing with PyYAML and the results are interesting. Seems PyYAML simply doesn't interpret 58899e0 as a number:

>>> import yaml
>>> config = {}
>>> config["version"] = '58899e0'
>>> yaml.dump(config, default_flow_style=False)
'version: 58899e0\n'
>>> test = yaml.dump(config, default_flow_style=False)
>>> test_config = yaml.load(test, Loader=yaml.CLoader)
>>> test_config['version']
'58899e0'

Which makes me confused how supervisor is, I assumed it used the same library? I think one of these two is wrong, either PyYAML should be treating 58899e0 like a number or supervisor shouldn't. This action seems to be doing it right and is stuck between the two.

@frenck
Copy link
Member

frenck commented Nov 18, 2021

Which makes me confused how supervisor is, I assumed it used the same library?

I think the supervisor uses the YAML 1.2 specifications right now, because of the YAML library it uses.

Bit of a bummer to have to convert back to JSON but seems like the only quick fix.

You don't have to convert anything back. Its only the output of the repository updater that changes back to JSON, the input can still be just YAML.

@mdegat01
Copy link
Author

Looking at the spec, YAML says what is a valid floating point number here. It provides a regex used to validate what is a valid floating point number in scientific notation:

-? [1-9] ( \. [0-9]* [1-9] )? ( e [-+] [1-9] [0-9]* )?

Notice that if the e is included it requires that be followed by either a + or - sign and a non-zero number. 58899e0 won't match that regex.
Screen Shot 2021-11-18 at 10 39 42 AM

So I think this might be a supervisor issue? Although I will note the spec is a bit confusing since it lists these as its examples of floating point numbers:

negative: !!float -1
zero: !!float 0
positive: !!float 2.3e4
infinity: !!float .inf
not a number: !!float .nan

Notice positive above doesn't match their regex either since e isn't followed by a + or - sign. But either way I don't think e0 is valid as a number.

@frenck
Copy link
Member

frenck commented Nov 18, 2021

It is definitely interpreted incorrectly, however, just using JSON output removes all that fuzz, so I'll go with that and call it a day.

@github-actions
Copy link

There hasn't been any activity on this issue recently, so we clean up some of the older and inactive issues.
Please make sure to update to the latest version and check if that solves the issue. Let us know if that works for you by leaving a comment 👍
This issue has now been marked as stale and will be closed if no further activity occurs. Thanks!

@github-actions github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label Dec 19, 2021
@frenck frenck added no-stale This issue or PR is exempted from the stable bot. stale There has not been activity on this issue or PR for quite some time. and removed stale There has not been activity on this issue or PR for quite some time. labels Dec 19, 2021
@github-actions github-actions bot removed the stale There has not been activity on this issue or PR for quite some time. label Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-stale This issue or PR is exempted from the stable bot.
Projects
None yet
Development

No branches or pull requests

2 participants