SDWAN-252 | authz version semver #1

katzoded · 2023-04-11T04:04:27Z

No description provided.

…ulong (ulong can be 32 bits in case it is not built with LP64. 2. add test to verify the max values of 64bits

… ulong (ulong can be 32 bits in case it is not built with LP64. 2. add test to verify the max values of 64bits

.github/pull_request_template.md

linear · 2023-04-11T04:05:58Z

SDWAN-252 [immediate-sd] - start using `authz-version` for OPA-less `immediate-SD`

.github/workflows/verify-pr.yml

CODEOWNERS

stablebits · 2023-04-11T08:35:44Z

include/semver/semver.hpp

    const std::string prerelease_allowed_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-";
-    const std::string version_pattern = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$";
-    const std::string loose_version_pattern = "^v?(0|[1-9]\\d*)(?:\\.(0|[1-9]\\d*))?(?:\\.(0|[1-9]\\d*))?(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$";
+    const std::string version_pattern = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)"


Why do we change regex? Where are these new patterns coming from?

It's the same regex, Oded just broke it into lines to make it easier to read the major.minor.patch parts.

@logan is right, I just broke the regex into those parts as defined in standard.

stablebits · 2023-04-11T08:39:16Z

include/semver/semver.hpp

-                    patch = patch_m.matched ? std::stoul(patch_m) : 0;
+                    major = (BuildIdentifierPart) parse_build_identifier_part(major_m);
+                    minor = minor_m.matched ? (BuildIdentifierPart) parse_build_identifier_part(minor_m) : 0;
+                    patch = patch_m.matched ? (BuildIdentifierPart) parse_build_identifier_part(patch_m) : 0;


parse_buld_identifier_part() already returns BuildIdentifierPart so we don't need these type casts.

right, good catch... its a leftovers

stablebits · 2023-04-11T08:53:12Z

include/semver/semver.hpp

 namespace semver
 {
+
+    typedef uint64_t BuildIdentifierPart;


minor: using is considered to be more in line with modern C++ than typedef. e.g. using BuildIdentifierPart = uint64_t;.

the idea was to allow users of this class to define the size of BuildIdentifierPart before the include (if someone wants a smaller size)

but for some reason I couldn't make it work...
I'll change it to using

stablebits · 2023-04-11T08:54:24Z

include/semver/semver.hpp


+    inline BuildIdentifierPart parse_build_identifier_part(const std::string& version_part)
+    {
+        return (BuildIdentifierPart)std::stoull(version_part);


minor: static_cast<BuildIdentifierPart>(...) is also more like modern C++ ))

katzoded · 2023-04-11T15:57:44Z

include/semver/semver.hpp

 namespace semver
 {
+
+    typedef uint64_t BuildIdentifierPart;


the idea was to allow users of this class to define the size of BuildIdentifierPart before the include (if someone wants a smaller size)

but for some reason I couldn't make it work...
I'll change it to using

katzoded · 2023-04-11T15:58:32Z

include/semver/semver.hpp

    const std::string prerelease_allowed_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz-";
-    const std::string version_pattern = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$";
-    const std::string loose_version_pattern = "^v?(0|[1-9]\\d*)(?:\\.(0|[1-9]\\d*))?(?:\\.(0|[1-9]\\d*))?(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$";
+    const std::string version_pattern = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)"


@logan is right, I just broke the regex into those parts as defined in standard.

katzoded · 2023-04-11T16:04:51Z

include/semver/semver.hpp

-    const std::string version_pattern = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$";
-    const std::string loose_version_pattern = "^v?(0|[1-9]\\d*)(?:\\.(0|[1-9]\\d*))?(?:\\.(0|[1-9]\\d*))?(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$";
+    const std::string version_pattern = "^(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)"
+                                        "(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))"


@Twingate/sdwan,
maybe you can help me understand this regex and if it complies with RFC ABNF .

<pre-release identifier> ::= <alphanumeric identifier> | <numeric identifier>

I would do something much easier on this one (?:-[a-zA-Z0-9\.]*), although I tried to modify it and it didn't go that easy for the tests!

actually the regex it taken from RFC

katzoded · 2023-04-11T16:05:35Z

include/semver/semver.hpp

-                    patch = patch_m.matched ? std::stoul(patch_m) : 0;
+                    major = (BuildIdentifierPart) parse_build_identifier_part(major_m);
+                    minor = minor_m.matched ? (BuildIdentifierPart) parse_build_identifier_part(minor_m) : 0;
+                    patch = patch_m.matched ? (BuildIdentifierPart) parse_build_identifier_part(patch_m) : 0;


right, good catch... its a leftovers

stablebits · 2023-04-11T19:13:09Z

include/semver/semver.hpp

 namespace semver
 {
+
+    using BuildIdentifierPart = uint64_t;


btw., this code is using variable_name style, not VariableStyle, for naming variables, functions, and types. It's better to use the same style when modifying existing code so that code looks consistent.

katzoded added 5 commits April 10, 2023 19:15

1. changed each version-indentifier-part to be a uint64_t instead of …

31fb88c

…ulong (ulong can be 32 bits in case it is not built with LP64. 2. add test to verify the max values of 64bits

first develop commit

9f01d50

1. changed each version-indentifier-part to be a uint64_t instead of…

3c4a057

… ulong (ulong can be 32 bits in case it is not built with LP64. 2. add test to verify the max values of 64bits

1. changed each version-indentifier-part to be a uint64_t instead of…

810fa9f

… ulong (ulong can be 32 bits in case it is not built with LP64. 2. add test to verify the max values of 64bits

add code owners

d78bc56

katzoded assigned stablebits Apr 11, 2023

ekampf reviewed Apr 11, 2023

View reviewed changes

.github/pull_request_template.md Outdated Show resolved Hide resolved

ekampf reviewed Apr 11, 2023

View reviewed changes

.github/workflows/verify-pr.yml Outdated Show resolved Hide resolved

ekampf reviewed Apr 11, 2023

View reviewed changes

CODEOWNERS Outdated Show resolved Hide resolved

katzoded added 2 commits April 10, 2023 21:08

remove redundant

28aad5a

remove the exception part from the parse function

46c2a5c

katzoded requested a review from ekampf April 11, 2023 04:10

katzoded unassigned stablebits Apr 11, 2023

katzoded requested a review from stablebits April 11, 2023 04:11

refactor parsing function name

a8479bd

stablebits reviewed Apr 11, 2023

View reviewed changes

katzoded commented Apr 11, 2023

View reviewed changes

address PR comments

3bedf65

katzoded requested review from loganbauer and stablebits April 11, 2023 18:19

stablebits reviewed Apr 11, 2023

View reviewed changes

stablebits approved these changes Apr 11, 2023

View reviewed changes

address PR comments

07ccb14

katzoded merged commit f614975 into main Apr 11, 2023

katzoded deleted the feature/ok/sdwan-252-authz-version-semver branch April 11, 2023 19:24

SDWAN-252 | authz version semver #1

SDWAN-252 | authz version semver #1

Uh oh!

Conversation

katzoded commented Apr 11, 2023

Uh oh!

Uh oh!

linear bot commented Apr 11, 2023

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

katzoded Apr 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

katzoded Apr 11, 2023 •

edited

Loading