Metadata Attribute research: spec_version

[MVrachev]

This issue aims to document thoughts about the spec_version attribute from the Signed class in order
to understand how we use that attribute, what might go wrong with it and how we can validate it.

We want to answer/address the following 6 questions/points based on my comment here:

1. How do we use it?
2. What might go wrong?
3. Rethink how we store it?
4. If, after the changes in step 3 there are concerns, then add a validation function
5. Make the necessary changes for points 1 - 4, think and propose a way to integrate it into the class/classes (decorators, descriptors, etc.)
6. create a pr for all additional changes on top of the validation functions

PS: The 7-th point is covered by documenting this issue.

[MVrachev]

My initial comments on those 6 steps:

1. How do we use it?

• Initialize it through init.

2. What might go wrong?

• Wrong or unsupported spec_version passed during initialization.
• Modification or deletion of the spec_version attribute directly from the metadata API, like this metadata.signed.spec_version = "2.10.10".

3. Rethink how we store it?

• spec_version cannot be immutable, because it’s set during initialization.
  After initialization, we want to discourage our users from changing it, by not allowing them to change it directly like metadata.signed.spec_version = "2.10.10".
  We can achieve this by making spec_version a property that sets a private version _spec_version
  and makes sure it’s set only once, so the user can’t do metadata.signed.spec_version = "2.10.10" later.
  Sadly, I didn’t find a way to block the user from changing _spec_version directly like:
  metadata.signed._spec_version = "2.10.10".

4. If, after the changes in step 3 there are concerns, then add a validation function

• We will need a validation function to validate that spec_version is a string in a semantic versioning format during initialization.

5. Make the necessary changes for points 1 - 4, think and propose a way to integrate it into the class/classes (decorators, descriptors, etc.)

• The validation function could be called into the spec_version.setter (spec_version is defined as a property)
  just before assigning the value to _spec_version.
  That way we make sure we won’t forget to validate the value when setting _spec_version during initialization.

6. create a pr for all additional changes on top of the validation functions

• I made a simple commit where small prototypes for the different attributes can be seen https://github.com/MVrachev/tuf/commits/checks

[MVrachev]

Initialize it through init.

@joshuagl commented the above:

It might be useful to add a default value of the currently supported spec version?
i.e. if I am creating a new Metadata object, I want the spec_version to be what the library supports.
Otherwise, I want the version in the file I am loading.
Does it make sense to change spec_version otherwise?

I answered:

Maybe not, but "spec_version" is not optional, so you should provide it either way. 
Maybe this is something we can think about for the specification? 
Some special value which will mean "spec_version = latest code_spec version"?

[MVrachev]

We can achieve this by making spec_version a property that sets a private version _spec_version

@sechkova commented the above:

I do agree we should make the attributes private and provide getters (@Property)
but we should not implement a public setter ( @spec_version.setter) when we don't need one.
spec_version is set only internally during init.

I commented back:

I agree with you that from those two options:

1. Either I make "spec_version" a property with a setter and thus I automatically call "_validate_spec_version()" 
on assignment.
2. or I make "spec_version" a read-only property, but remember to call "_validate_spec_version"
everywhere we change "spec_version". 
Here, actually, I won't change "self.spec_version", but will actually change "self.spec_version".

The second option is better for spec_version because I set up "spec_version" only during initialization.

[MVrachev]

We will need a validation function to validate that spec_version is a string in a semantic versioning format during initialization.

@jku commented the above:

if the validation is short and only needed in one place,
the validation code should be there and not in a specific validation function IMO

Validating semantic version completely is bonkers (it's really complex), I suggest we explicitly do not do it.
I see two reasonable choices:
* let's just check that we can split the string in three dot separated parts and that the first part parses as int
* make extra demands that are not in the spec and say that all parts of semver must be numbers, and validate that

it seems your implementation is option 2 in my comment...

this is valid semver 1.0.0-beta+exp.sha.5114f85 -- sure, it does not really make any sense as TUF spec version
but I think we can also allow it without parsing it (since we're only interested in MAJOR version)...