Skip to content

pkg/aaparser: support parsing version like "3.0.0-beta1"#41518

Merged
tianon merged 1 commit intomoby:masterfrom
AkihiroSuda:fix-41517
Oct 7, 2020
Merged

pkg/aaparser: support parsing version like "3.0.0-beta1"#41518
tianon merged 1 commit intomoby:masterfrom
AkihiroSuda:fix-41517

Conversation

@AkihiroSuda
Copy link
Copy Markdown
Member

Fix #41517

Fix moby#41517

Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah
Copy link
Copy Markdown
Member

@cpuguy83 @tianon ptal

@mohamedGamalAbuGalala
Copy link
Copy Markdown

Will this be merged soon?

@tianon tianon merged commit e246a85 into moby:master Oct 7, 2020
tianon added a commit to tianon/debian-docker that referenced this pull request Oct 8, 2020
version := words[len(words)-1]

// trim "-beta1" suffix from version="3.0.0-beta1" if exists
version = strings.SplitN(version, "-", 2)[0]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@alexmurray this is pretty similar to what you applied in Ubuntu (~ one month ago 😅) -- is there a specific reason you'd decided to also trim ~... in addition to/after -...? 🙏

For reference (slash to jog your memory), here's the patch you ended up with:

Description: Handle apparmor versions like 3.0.0-beta1 appropriately
Author: Alex Murray <alex.murray@canonical.com>
--- a/components/engine/pkg/aaparser/aaparser.go
+++ b/components/engine/pkg/aaparser/aaparser.go
@@ -55,6 +55,8 @@ func parseVersion(output string) (int, e
 	lines := strings.SplitN(output, "\n", 2)
 	words := strings.Split(lines[0], " ")
 	version := words[len(words)-1]
+	version = strings.SplitN(version, "-", 2)[0]
+	version = strings.SplitN(version, "~", 2)[0]
 
 	// split by major minor version
 	v := strings.Split(version, ".")

cc @mwhudson

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The upstream AppArmor maintainer mentioned that sometimes they have used a tilde ~ in the past and so it would be more future proof to include this as well.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Perfect, thank you! Found a good example in https://gitlab.com/apparmor/apparmor/-/commit/bca67d3d27d219d11ce8c9cc70612bd637f88c10 (and will be sending a follow-up PR to include that other half shortly). 👍 ❤️

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.

Parsing AppArmor version fails on Ubuntu 20.10

5 participants