From 2fdb2ee486ffa968fe99d1aca3927f17733838cc Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Sun, 11 Aug 2019 14:48:07 +0300 Subject: [PATCH 01/14] PF-4 Add check for HTTPS access for sources --- check/checkers.go | 54 ++++++++++++++++++++++++++++++ check/checkers_test.go | 16 ++++++++- cli/cli.go | 2 +- common/perfecto.spec | 5 ++- spec/spec.go | 54 ++++++++++++++++++++++-------- spec/spec_test.go | 19 ++++++++--- testdata/test.spec | 3 ++ testdata/test_11.spec | 75 ++++++++++++++++++++++++++++++++++++++++++ 8 files changed, 208 insertions(+), 20 deletions(-) create mode 100644 testdata/test_11.spec diff --git a/check/checkers.go b/check/checkers.go index 4437db9..d68ea4b 100644 --- a/check/checkers.go +++ b/check/checkers.go @@ -12,6 +12,7 @@ import ( "regexp" "strings" + "pkg.re/essentialkaos/ek.v10/req" "pkg.re/essentialkaos/ek.v10/sliceutil" "pkg.re/essentialkaos/ek.v10/strutil" @@ -86,6 +87,7 @@ func getCheckers() []Checker { checkForSetupArguments, checkForEmptyLinesAtEnd, checkBashLoops, + checkURLForHTTPS, } } @@ -691,6 +693,42 @@ func checkBashLoops(s *spec.Spec) []Alert { return result } +// checkURLForHTTPS checks if source domain supports HTTPS +func checkURLForHTTPS(s *spec.Spec) []Alert { + if len(s.Data) == 0 { + return nil + } + + sources := s.GetSources() + + var result []Alert + + for _, line := range sources { + sourceText := strings.TrimLeft(line.Text, "\t ") + sourceURL := strutil.ReadField(sourceText, 1, true, " ") + + if !strings.HasPrefix(sourceURL, "http://") { + continue + } + + sourceDomain := extractSourceDomain(sourceURL) + + if sourceDomain == "" { + continue + } + + if isHostSupportsHTTPS(sourceDomain) { + result = append(result, Alert{ + LEVEL_WARNING, + fmt.Sprintf("Domain %s supports HTTPS. Replace http by https in source URL.", sourceDomain), + line, + }) + } + } + + return result +} + // ////////////////////////////////////////////////////////////////////////////////// // // prefix is strings.HasPrefix wrapper @@ -745,3 +783,19 @@ func containsTag(data []spec.Line, tag string) bool { return false } + +// extractSourceDomain extracts domain name from source URL +func extractSourceDomain(url string) string { + url = strutil.Exclude(url, "http://") + return strutil.ReadField(url, 0, false, "/") +} + +// isHostSupportsHTTPS return true if domain supports HTTPS protocol +func isHostSupportsHTTPS(domain string) bool { + _, err := req.Request{ + URL: "https://" + domain, + AutoDiscard: true, + }.Head() + + return err == nil +} diff --git a/check/checkers_test.go b/check/checkers_test.go index 9be91b2..3558f85 100644 --- a/check/checkers_test.go +++ b/check/checkers_test.go @@ -340,6 +340,19 @@ func (sc *CheckSuite) TestCheckBashLoops(c *chk.C) { c.Assert(alerts[1].Line.Index, chk.Equals, 49) } +func (sc *CheckSuite) TestCheckURLForHTTPS(c *chk.C) { + s, err := spec.Read("../testdata/test_11.spec") + + c.Assert(err, chk.IsNil) + c.Assert(s, chk.NotNil) + + alerts := checkURLForHTTPS(s) + + c.Assert(alerts, chk.HasLen, 1) + c.Assert(alerts[0].Info, chk.Equals, "Domain kaos.st supports HTTPS. Replace http by https in source URL.") + c.Assert(alerts[0].Line.Index, chk.Equals, 13) +} + func (sc *CheckSuite) TestWithEmptyData(c *chk.C) { s := &spec.Spec{} @@ -362,6 +375,7 @@ func (sc *CheckSuite) TestWithEmptyData(c *chk.C) { c.Assert(checkForSetupArguments(s), chk.IsNil) c.Assert(checkForEmptyLinesAtEnd(s), chk.IsNil) c.Assert(checkBashLoops(s), chk.IsNil) + c.Assert(checkURLForHTTPS(s), chk.IsNil) } func (sc *CheckSuite) TestRPMLint(c *chk.C) { @@ -422,7 +436,7 @@ func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { func (sc *CheckSuite) TestAux(c *chk.C) { // This test will fail if new checkers was added - c.Assert(getCheckers(), chk.HasLen, 19) + c.Assert(getCheckers(), chk.HasLen, 20) r := &Report{} c.Assert(r.IsPerfect(), chk.Equals, true) diff --git a/cli/cli.go b/cli/cli.go index 54d1515..0a25e1e 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -32,7 +32,7 @@ import ( // App info const ( APP = "Perfecto" - VER = "2.4.0" + VER = "2.5.0" DESC = "Tool for checking perfectly written RPM specs" ) diff --git a/common/perfecto.spec b/common/perfecto.spec index eb0195e..0fa00b5 100644 --- a/common/perfecto.spec +++ b/common/perfecto.spec @@ -10,7 +10,7 @@ Summary: Tool for checking perfectly written RPM specs Name: perfecto -Version: 2.4.0 +Version: 2.5.0 Release: 0%{?dist} Group: Development/Tools License: EKOL @@ -87,6 +87,9 @@ fi ################################################################################ %changelog +* Sun Aug 11 2019 Anton Novojilov - 2.5.0-0 +- Added check for HTTPS support on a source domain + * Tue Jul 09 2019 Anton Novojilov - 2.4.0-0 - Added check for bash loops syntax - Added completions generators diff --git a/spec/spec.go b/spec/spec.go index e4437ff..80cb939 100644 --- a/spec/spec.go +++ b/spec/spec.go @@ -164,6 +164,11 @@ func (s *Spec) GetHeaders() []*Header { return extractHeaders(s) } +// GetSources returns slice with sources +func (s *Spec) GetSources() []Line { + return extractSources(s) +} + // GetLine return spec line by index func (s *Spec) GetLine(index int) Line { if index < 0 { @@ -199,7 +204,7 @@ func (s *Section) GetPackageName() string { // ////////////////////////////////////////////////////////////////////////////////// // -// readFile read and parse spec file +// readFile reads and parses spec file func readFile(file string) (*Spec, error) { fd, err := os.OpenFile(file, os.O_RDONLY, 0) @@ -250,7 +255,7 @@ LOOP: return spec, nil } -// checkFile check file for errors +// checkFile checks file for errors func checkFile(file string) error { if !fsutil.IsExist(file) { return fmt.Errorf("File %s doesn't exist", file) @@ -271,7 +276,7 @@ func checkFile(file string) error { return nil } -// hasSection return true if spec contains given section +// hasSection returns true if spec contains given section func hasSection(s *Spec, sectionName string) bool { for _, line := range s.Data { if strings.HasPrefix(line.Text, "%"+sectionName) { @@ -282,7 +287,7 @@ func hasSection(s *Spec, sectionName string) bool { return false } -// extractSections extract data for given sections +// extractSections extracts data for given sections func extractSections(s *Spec, names []string) []*Section { var result []*Section var section *Section @@ -323,7 +328,7 @@ func extractSections(s *Spec, names []string) []*Section { return result } -// extractHeaders extract packages' headers +// extractHeaders extracts packages' headers func extractHeaders(s *Spec) []*Header { var result []*Header var header *Header @@ -355,7 +360,30 @@ func extractHeaders(s *Spec) []*Header { return result } -// isSectionHeader return if given string is package header +// extractSources extracts sources from spec file +func extractSources(s *Spec) []Line { + var result []Line + + for _, line := range s.Data { + if line.Skip { + continue + } + + if isSectionHeader(line.Text) { + break + } + + lineText := strings.TrimLeft(line.Text, "\t ") + + if strings.HasPrefix(lineText, "Source") { + result = append(result, line) + } + } + + return result +} + +// isSectionHeader returns if given string is package header func isSectionHeader(text string) bool { for _, sectionName := range sections { if strings.HasPrefix(text, "%"+sectionName) { @@ -366,7 +394,7 @@ func isSectionHeader(text string) bool { return false } -// isHeaderTag return if given string is header tag +// isHeaderTag returns if given string is header tag func isHeaderTag(text string) bool { for _, tagName := range tags { if strings.HasPrefix(text, tagName) { @@ -377,7 +405,7 @@ func isHeaderTag(text string) bool { return false } -// parseSectionName parse section name +// parseSectionName parses section name func parseSectionName(text string) (string, []string) { if !strings.Contains(text, " ") { return strings.TrimLeft(text, "%"), nil @@ -388,7 +416,7 @@ func parseSectionName(text string) (string, []string) { return strings.TrimLeft(sectionNameSlice[0], "%"), sectionNameSlice[1:] } -// parsePackageName parse package name +// parsePackageName parses package name func parsePackageName(text string) (string, bool) { if strutil.ReadField(text, 1, true) == "-n" { return strutil.ReadField(text, 2, true), false @@ -397,7 +425,7 @@ func parsePackageName(text string) (string, bool) { return strutil.ReadField(text, 1, true), true } -// isSectionMatch return true if data contains name of any given sections +// isSectionMatch returns true if data contains name of any given sections func isSectionMatch(data string, names []string) bool { if len(names) == 0 { return true @@ -412,7 +440,7 @@ func isSectionMatch(data string, names []string) bool { return false } -// isSpec check that given file contains spec data +// isSpec checks that given file contains spec data func isSpec(spec *Spec) bool { var count int @@ -444,12 +472,12 @@ func isSpec(spec *Spec) bool { return count >= 3 } -// isSkipTag return true if text contains skip tag +// isSkipTag returns true if text contains skip tag func isSkipTag(text string) bool { return strings.Contains(text, "perfecto:absolve") } -// extractSkipCount return number of lines to skip +// extractSkipCount returns number of lines to skip func extractSkipCount(text string) int { count := strutil.ReadField(text, 2, true) diff --git a/spec/spec_test.go b/spec/spec_test.go index db6007f..b9a2779 100644 --- a/spec/spec_test.go +++ b/spec/spec_test.go @@ -50,7 +50,7 @@ func (s *SpecSuite) TestParsing(c *C) { c.Assert(spec.GetLine(-1), DeepEquals, Line{-1, "", false}) c.Assert(spec.GetLine(99), DeepEquals, Line{-1, "", false}) - c.Assert(spec.GetLine(36), DeepEquals, Line{36, "%{__make} %{?_smp_mflags}", false}) + c.Assert(spec.GetLine(39), DeepEquals, Line{39, "%{__make} %{?_smp_mflags}", false}) } func (s *SpecSuite) TestSections(c *C) { @@ -67,8 +67,8 @@ func (s *SpecSuite) TestSections(c *C) { sections = spec.GetSections(SECTION_BUILD) c.Assert(sections, HasLen, 1) c.Assert(sections[0].Data, HasLen, 2) - c.Assert(sections[0].Start, Equals, 35) - c.Assert(sections[0].End, Equals, 37) + c.Assert(sections[0].Start, Equals, 38) + c.Assert(sections[0].End, Equals, 40) sections = spec.GetSections(SECTION_SETUP) c.Assert(sections[0].Name, Equals, "setup") c.Assert(sections[0].Args, DeepEquals, []string{"-qn", "%{name}-%{version}"}) @@ -88,7 +88,7 @@ func (s *SpecSuite) TestHeaders(c *C) { c.Assert(headers, HasLen, 2) c.Assert(headers[0].Package, Equals, "") c.Assert(headers[0].IsSubpackage, Equals, false) - c.Assert(headers[0].Data, HasLen, 13) + c.Assert(headers[0].Data, HasLen, 16) c.Assert(headers[1].Package, Equals, "magic") c.Assert(headers[1].IsSubpackage, Equals, true) c.Assert(headers[1].Data, HasLen, 4) @@ -101,6 +101,17 @@ func (s *SpecSuite) TestHeaders(c *C) { c.Assert(subPkg, Equals, false) } +func (s *SpecSuite) TestSourceExtractor(c *C) { + spec, err := Read("../testdata/test.spec") + + c.Assert(err, IsNil) + c.Assert(spec, NotNil) + + sources := spec.GetSources() + + c.Assert(sources, HasLen, 1) +} + func (s *SpecSuite) TestSkipTag(c *C) { c.Assert(isSkipTag("# perfecto:absolve 3"), Equals, true) c.Assert(isSkipTag("# abcd 1"), Equals, false) diff --git a/testdata/test.spec b/testdata/test.spec index 1a46a92..1310d02 100644 --- a/testdata/test.spec +++ b/testdata/test.spec @@ -12,6 +12,9 @@ BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} - Source0: https://domain.com/%{name}-%{version}.tar.gz +# perfecto:absolve +Source1: http://domain.com/%{name}-%{version}.tar.gz + ################################################################################ %description diff --git a/testdata/test_11.spec b/testdata/test_11.spec new file mode 100644 index 0000000..8ae0575 --- /dev/null +++ b/testdata/test_11.spec @@ -0,0 +1,75 @@ +################################################################################ + +Summary: Test spec for perfecto +Name: perfecto-spec +Version: 1.0.0 +Release: 0%{?dist} +Group: System Environment/Base +License: MIT +URL: https://domain.com + +BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) + +Source0: http://kaos.st/%{name}-%{version}-1.tar.gz + +# perfecto:absolve +Source1: http://kaos.st/%{name}-%{version}-2.tar.gz +Source2: %{name}-%{version}-3.tar.gz +Source3: http:// + +################################################################################ + +%description +Test spec for perfecto app. + +################################################################################ + +%package magic + +Summary: Test subpackage for perfecto +Group: System Environment/Base + +%description magic +Test subpackge for perfecto app. + +################################################################################ + +%prep +%setup -qn %{name}-%{version} + +%build +%{__make} %{?_smp_mflags} + +%install +rm -rf %{buildroot} + +%{make_install} PREFIX=%{buildroot}%{_prefix} + +%clean +# perfecto:absolve 2 +rm -rf %{buildroot} + +%post +%{__chkconfig} --add %{name} &>/dev/null || : + +%preun +%{__chkconfig} --del %{name} &> /dev/null || : + +%postun +%{__chkconfig} --del %{name} &> /dev/null || : + +################################################################################ + +%files +%defattr(-,root,root,-) +%{_bindir}/%{name} + +%files magic +%defattr(-,root,root,-) +%{_bindir}/%{name}-magic + +################################################################################ + +%changelog +* Wed Jan 24 2018 Anton Novojilov - 1.0.0-0 +- Test changelog record From 4be5be7318c5a15ba95f2fc0c2a45d6f38b2da0a Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Sun, 11 Aug 2019 15:08:25 +0300 Subject: [PATCH 02/14] Improved tests for checkers --- check/checkers_test.go | 57 ++++++++++++++++++++++++------------------ 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/check/checkers_test.go b/check/checkers_test.go index 3558f85..a258cad 100644 --- a/check/checkers_test.go +++ b/check/checkers_test.go @@ -408,30 +408,39 @@ func (sc *CheckSuite) TestRPMLint(c *chk.C) { } func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { - i, s1, s2 := extractAlertData("test.spec: W: no-buildroot-tag") - c.Assert(i, chk.Equals, -1) - c.Assert(s1, chk.Equals, "W") - c.Assert(s2, chk.Equals, "no-buildroot-tag") - - i, s1, s2 = extractAlertData("test.spec: E: specfile-error error: line 356: Unknown tag: Release1") - c.Assert(i, chk.Equals, 356) - c.Assert(s1, chk.Equals, "E") - c.Assert(s2, chk.Equals, "Unknown tag: Release1") - - i, s1, s2 = extractAlertData("test.spec:67: W: macro-in-%changelog %record") - c.Assert(i, chk.Equals, 67) - c.Assert(s1, chk.Equals, "W") - c.Assert(s2, chk.Equals, "macro-in-%changelog %record") - - i, s1, s2 = extractAlertData("test.spec: E: specfile-error error: line A: Unknown tag: Release1") - c.Assert(i, chk.Equals, -1) - c.Assert(s1, chk.Equals, "") - c.Assert(s2, chk.Equals, "") - - i, s1, s2 = extractAlertData("test.spec:A: W: macro-in-%changelog %record") - c.Assert(i, chk.Equals, -1) - c.Assert(s1, chk.Equals, "") - c.Assert(s2, chk.Equals, "") + s, err := spec.Read("../testdata/test_7.spec") + + c.Assert(s, chk.NotNil) + c.Assert(err, chk.IsNil) + + a, ok := parseAlertLine("test.spec: W: no-buildroot-tag", s) + + c.Assert(ok, chk.Equals, true) + c.Assert(a.Level, chk.Equals, LEVEL_ERROR) + c.Assert(a.Info, chk.Equals, "[rpmlint] no-buildroot-tag") + c.Assert(a.Line.Index, chk.Equals, -1) + + a, ok = parseAlertLine("test.spec: E: specfile-error error: line 10: Unknown tag: Release1", s) + + c.Assert(ok, chk.Equals, true) + c.Assert(a.Level, chk.Equals, LEVEL_CRITICAL) + c.Assert(a.Info, chk.Equals, "[rpmlint] Unknown tag: Release1") + c.Assert(a.Line.Index, chk.Equals, 10) + + a, ok = parseAlertLine("test.spec:67: W: macro-in-%changelog %record", s) + + c.Assert(ok, chk.Equals, true) + c.Assert(a.Level, chk.Equals, LEVEL_ERROR) + c.Assert(a.Info, chk.Equals, "[rpmlint] macro-in-%changelog %record") + c.Assert(a.Line.Index, chk.Equals, 67) + + a, ok = parseAlertLine("test.spec: E: specfile-error error: line A: Unknown tag: Release1", s) + + c.Assert(ok, chk.Equals, false) + + a, ok = parseAlertLine("test.spec:A: W: macro-in-%changelog %record", s) + + c.Assert(ok, chk.Equals, false) } func (sc *CheckSuite) TestAux(c *chk.C) { From 2b651dd250f1f233b4293376b560b05b84145e0e Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Sun, 11 Aug 2019 15:26:17 +0300 Subject: [PATCH 03/14] Fixed tests compatibility with old version on rpmlint on TravisCI --- check/check.go | 7 +++---- check/checkers_test.go | 11 +++++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/check/check.go b/check/check.go index bbead5c..635583e 100644 --- a/check/check.go +++ b/check/check.go @@ -77,7 +77,8 @@ func Check(s *spec.Spec, lint bool, linterConfig string) *Report { checkers := getCheckers() if lint { - appendLinterAlerts(s, report, linterConfig) + alerts := Lint(s, linterConfig) + appendLinterAlerts(report, alerts) } for _, checker := range checkers { @@ -110,9 +111,7 @@ func Check(s *spec.Spec, lint bool, linterConfig string) *Report { // ////////////////////////////////////////////////////////////////////////////////// // // appendLinterAlerts append rpmlint alerts to report -func appendLinterAlerts(s *spec.Spec, r *Report, linterConfig string) { - alerts := Lint(s, linterConfig) - +func appendLinterAlerts(r *Report, alerts []Alert) { if len(alerts) == 0 { return } diff --git a/check/checkers_test.go b/check/checkers_test.go index a258cad..e9c1b72 100644 --- a/check/checkers_test.go +++ b/check/checkers_test.go @@ -408,6 +408,9 @@ func (sc *CheckSuite) TestRPMLint(c *chk.C) { } func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { + report := &Report{} + alerts := []Alert{} + s, err := spec.Read("../testdata/test_7.spec") c.Assert(s, chk.NotNil) @@ -419,6 +422,7 @@ func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { c.Assert(a.Level, chk.Equals, LEVEL_ERROR) c.Assert(a.Info, chk.Equals, "[rpmlint] no-buildroot-tag") c.Assert(a.Line.Index, chk.Equals, -1) + alerts = append(alerts, a) a, ok = parseAlertLine("test.spec: E: specfile-error error: line 10: Unknown tag: Release1", s) @@ -426,6 +430,7 @@ func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { c.Assert(a.Level, chk.Equals, LEVEL_CRITICAL) c.Assert(a.Info, chk.Equals, "[rpmlint] Unknown tag: Release1") c.Assert(a.Line.Index, chk.Equals, 10) + alerts = append(alerts, a) a, ok = parseAlertLine("test.spec:67: W: macro-in-%changelog %record", s) @@ -433,6 +438,7 @@ func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { c.Assert(a.Level, chk.Equals, LEVEL_ERROR) c.Assert(a.Info, chk.Equals, "[rpmlint] macro-in-%changelog %record") c.Assert(a.Line.Index, chk.Equals, 67) + alerts = append(alerts, a) a, ok = parseAlertLine("test.spec: E: specfile-error error: line A: Unknown tag: Release1", s) @@ -441,6 +447,11 @@ func (sc *CheckSuite) TestRPMLintParser(c *chk.C) { a, ok = parseAlertLine("test.spec:A: W: macro-in-%changelog %record", s) c.Assert(ok, chk.Equals, false) + + appendLinterAlerts(report, alerts) + + c.Assert(report.Errors, chk.HasLen, 2) + c.Assert(report.Criticals, chk.HasLen, 1) } func (sc *CheckSuite) TestAux(c *chk.C) { From 4fe2021de3922d1b8199dd8697d2745aeae75bc1 Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Tue, 13 Aug 2019 17:41:13 +0300 Subject: [PATCH 04/14] hadolint updated to 1.17.1 --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 8df63c4..f79a237 100644 --- a/.travis.yml +++ b/.travis.yml @@ -23,7 +23,7 @@ cache: apt before_install: - wget https://storage.googleapis.com/shellcheck/shellcheck-latest.linux.x86_64.tar.xz - - wget -O hadolint https://github.com/hadolint/hadolint/releases/download/v1.15.0/hadolint-Linux-x86_64 + - wget -O hadolint https://github.com/hadolint/hadolint/releases/download/v1.17.1/hadolint-Linux-x86_64 - tar xf shellcheck-latest.linux.x86_64.tar.xz - echo "deb http://us.archive.ubuntu.com/ubuntu xenial main universe" | sudo tee -a /etc/apt/sources.list - sudo apt-get update -qq From c2b93437b49df4ef3dcf1273dc3834100157875e Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 02:31:23 +0300 Subject: [PATCH 05/14] ek package updated to the latest version --- .travis.yml | 3 ++- Makefile | 6 +++--- check/checkers.go | 6 +++--- check/rpmlint.go | 2 +- cli/cli.go | 24 ++++++++++++------------ cli/render.go | 6 +++--- common/perfecto.spec | 7 +++++-- spec/spec.go | 6 +++--- 8 files changed, 32 insertions(+), 28 deletions(-) diff --git a/.travis.yml b/.travis.yml index f79a237..f670ead 100644 --- a/.travis.yml +++ b/.travis.yml @@ -4,6 +4,7 @@ go: - 1.10.x - 1.11.x - 1.12.x + - 1.13.x - tip os: @@ -23,7 +24,7 @@ cache: apt before_install: - wget https://storage.googleapis.com/shellcheck/shellcheck-latest.linux.x86_64.tar.xz - - wget -O hadolint https://github.com/hadolint/hadolint/releases/download/v1.17.1/hadolint-Linux-x86_64 + - wget -O hadolint https://github.com/hadolint/hadolint/releases/download/v1.17.2/hadolint-Linux-x86_64 - tar xf shellcheck-latest.linux.x86_64.tar.xz - echo "deb http://us.archive.ubuntu.com/ubuntu xenial main universe" | sudo tee -a /etc/apt/sources.list - sudo apt-get update -qq diff --git a/Makefile b/Makefile index 36bca0f..46280c4 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ ################################################################################ -# This Makefile generated by GoMakeGen 1.1.0 using next command: +# This Makefile generated by GoMakeGen 1.2.0 using next command: # gomakegen . # # More info: https://kaos.sh/gomakegen @@ -27,7 +27,7 @@ git-config: ## Configure git redirects for stable import path services git config --global http.https://pkg.re.followRedirects true deps: git-config ## Download dependencies - go get -d -v pkg.re/essentialkaos/ek.v10 + go get -d -v pkg.re/essentialkaos/ek.v11 deps-test: git-config ## Download dependencies for tests go get -d -v pkg.re/check.v1 @@ -46,6 +46,6 @@ help: ## Show this info @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) \ | awk 'BEGIN {FS = ":.*?## "}; {printf " \033[33m%-12s\033[0m %s\n", $$1, $$2}' @echo -e '' - @echo -e '\033[90mGenerated by GoMakeGen 1.1.0\033[0m\n' + @echo -e '\033[90mGenerated by GoMakeGen 1.2.0\033[0m\n' ################################################################################ diff --git a/check/checkers.go b/check/checkers.go index d68ea4b..deebe1f 100644 --- a/check/checkers.go +++ b/check/checkers.go @@ -12,9 +12,9 @@ import ( "regexp" "strings" - "pkg.re/essentialkaos/ek.v10/req" - "pkg.re/essentialkaos/ek.v10/sliceutil" - "pkg.re/essentialkaos/ek.v10/strutil" + "pkg.re/essentialkaos/ek.v11/req" + "pkg.re/essentialkaos/ek.v11/sliceutil" + "pkg.re/essentialkaos/ek.v11/strutil" "github.com/essentialkaos/perfecto/spec" ) diff --git a/check/rpmlint.go b/check/rpmlint.go index f81c22b..392d84c 100644 --- a/check/rpmlint.go +++ b/check/rpmlint.go @@ -12,7 +12,7 @@ import ( "strconv" "strings" - "pkg.re/essentialkaos/ek.v10/strutil" + "pkg.re/essentialkaos/ek.v11/strutil" "github.com/essentialkaos/perfecto/spec" ) diff --git a/cli/cli.go b/cli/cli.go index 0a25e1e..c685c99 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -11,17 +11,17 @@ import ( "fmt" "os" - "pkg.re/essentialkaos/ek.v10/env" - "pkg.re/essentialkaos/ek.v10/fmtc" - "pkg.re/essentialkaos/ek.v10/mathutil" - "pkg.re/essentialkaos/ek.v10/options" - "pkg.re/essentialkaos/ek.v10/sliceutil" - "pkg.re/essentialkaos/ek.v10/strutil" - "pkg.re/essentialkaos/ek.v10/usage" - "pkg.re/essentialkaos/ek.v10/usage/completion/bash" - "pkg.re/essentialkaos/ek.v10/usage/completion/fish" - "pkg.re/essentialkaos/ek.v10/usage/completion/zsh" - "pkg.re/essentialkaos/ek.v10/usage/update" + "pkg.re/essentialkaos/ek.v11/env" + "pkg.re/essentialkaos/ek.v11/fmtc" + "pkg.re/essentialkaos/ek.v11/mathutil" + "pkg.re/essentialkaos/ek.v11/options" + "pkg.re/essentialkaos/ek.v11/sliceutil" + "pkg.re/essentialkaos/ek.v11/strutil" + "pkg.re/essentialkaos/ek.v11/usage" + "pkg.re/essentialkaos/ek.v11/usage/completion/bash" + "pkg.re/essentialkaos/ek.v11/usage/completion/fish" + "pkg.re/essentialkaos/ek.v11/usage/completion/zsh" + "pkg.re/essentialkaos/ek.v11/usage/update" "github.com/essentialkaos/perfecto/check" "github.com/essentialkaos/perfecto/spec" @@ -32,7 +32,7 @@ import ( // App info const ( APP = "Perfecto" - VER = "2.5.0" + VER = "2.6.0" DESC = "Tool for checking perfectly written RPM specs" ) diff --git a/cli/render.go b/cli/render.go index 34c3d23..2c535fe 100644 --- a/cli/render.go +++ b/cli/render.go @@ -13,9 +13,9 @@ import ( "strconv" "strings" - "pkg.re/essentialkaos/ek.v10/fmtc" - "pkg.re/essentialkaos/ek.v10/options" - "pkg.re/essentialkaos/ek.v10/strutil" + "pkg.re/essentialkaos/ek.v11/fmtc" + "pkg.re/essentialkaos/ek.v11/options" + "pkg.re/essentialkaos/ek.v11/strutil" "github.com/essentialkaos/perfecto/check" "github.com/essentialkaos/perfecto/spec" diff --git a/common/perfecto.spec b/common/perfecto.spec index 0fa00b5..7af63b4 100644 --- a/common/perfecto.spec +++ b/common/perfecto.spec @@ -10,7 +10,7 @@ Summary: Tool for checking perfectly written RPM specs Name: perfecto -Version: 2.5.0 +Version: 2.6.0 Release: 0%{?dist} Group: Development/Tools License: EKOL @@ -20,7 +20,7 @@ Source0: https://source.kaos.st/%{name}/%{name}-%{version}.tar.bz2 BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -BuildRequires: golang >= 1.12 +BuildRequires: golang >= 1.13 Requires: rpmlint @@ -87,6 +87,9 @@ fi ################################################################################ %changelog +* Fri Oct 25 2019 Anton Novojilov - 2.6.0-0 +- ek package updated to the latest version + * Sun Aug 11 2019 Anton Novojilov - 2.5.0-0 - Added check for HTTPS support on a source domain diff --git a/spec/spec.go b/spec/spec.go index 80cb939..5f70b42 100644 --- a/spec/spec.go +++ b/spec/spec.go @@ -15,9 +15,9 @@ import ( "strconv" "strings" - "pkg.re/essentialkaos/ek.v10/fsutil" - "pkg.re/essentialkaos/ek.v10/path" - "pkg.re/essentialkaos/ek.v10/strutil" + "pkg.re/essentialkaos/ek.v11/fsutil" + "pkg.re/essentialkaos/ek.v11/path" + "pkg.re/essentialkaos/ek.v11/strutil" ) // ////////////////////////////////////////////////////////////////////////////////// // From c53c0d7cd8ee4c815b5c60b8552acff331a6511c Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 03:26:15 +0300 Subject: [PATCH 06/14] Added cache for HTTPS checks --- check/checkers.go | 22 +++++++++++++++++++--- common/perfecto.spec | 1 + 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/check/checkers.go b/check/checkers.go index deebe1f..dde0757 100644 --- a/check/checkers.go +++ b/check/checkers.go @@ -11,7 +11,9 @@ import ( "fmt" "regexp" "strings" + "time" + "pkg.re/essentialkaos/ek.v11/cache" "pkg.re/essentialkaos/ek.v11/req" "pkg.re/essentialkaos/ek.v11/sliceutil" "pkg.re/essentialkaos/ek.v11/strutil" @@ -24,13 +26,15 @@ import ( // Checker is spec check function type Checker func(s *spec.Spec) []Alert -// ////////////////////////////////////////////////////////////////////////////////// // - type macro struct { Value string Name string } +// ////////////////////////////////////////////////////////////////////////////////// // + +var httpCheckCache *cache.Store + var pathMacroSlice = []macro{ {"/etc/init", "%{_initddir}"}, {"/etc/rc.d/init.d", "%{_initddir}"}, @@ -699,6 +703,10 @@ func checkURLForHTTPS(s *spec.Spec) []Alert { return nil } + if httpCheckCache == nil { + httpCheckCache = cache.New(time.Minute, 0) + } + sources := s.GetSources() var result []Alert @@ -792,10 +800,18 @@ func extractSourceDomain(url string) string { // isHostSupportsHTTPS return true if domain supports HTTPS protocol func isHostSupportsHTTPS(domain string) bool { + if httpCheckCache.Has(domain) { + return httpCheckCache.Get(domain).(bool) + } + _, err := req.Request{ URL: "https://" + domain, AutoDiscard: true, }.Head() - return err == nil + supported := err == nil + + httpCheckCache.Set(domain, supported) + + return supported } diff --git a/common/perfecto.spec b/common/perfecto.spec index 7af63b4..eb8cd45 100644 --- a/common/perfecto.spec +++ b/common/perfecto.spec @@ -89,6 +89,7 @@ fi %changelog * Fri Oct 25 2019 Anton Novojilov - 2.6.0-0 - ek package updated to the latest version +- Added cache for HTTPS checks * Sun Aug 11 2019 Anton Novojilov - 2.5.0-0 - Added check for HTTPS support on a source domain From 43d9221069c843468c9e1947c3307adbd33004e8 Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 03:28:54 +0300 Subject: [PATCH 07/14] Fix version --- cli/cli.go | 2 +- common/perfecto.spec | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/cli/cli.go b/cli/cli.go index c685c99..32b19e6 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -32,7 +32,7 @@ import ( // App info const ( APP = "Perfecto" - VER = "2.6.0" + VER = "2.5.0" DESC = "Tool for checking perfectly written RPM specs" ) diff --git a/common/perfecto.spec b/common/perfecto.spec index eb8cd45..a63c3fb 100644 --- a/common/perfecto.spec +++ b/common/perfecto.spec @@ -87,12 +87,10 @@ fi ################################################################################ %changelog -* Fri Oct 25 2019 Anton Novojilov - 2.6.0-0 -- ek package updated to the latest version -- Added cache for HTTPS checks - -* Sun Aug 11 2019 Anton Novojilov - 2.5.0-0 +* Fri Oct 25 2019 Anton Novojilov - 2.5.0-0 - Added check for HTTPS support on a source domain +- Added cache for HTTPS checks +- ek package updated to the latest version * Tue Jul 09 2019 Anton Novojilov - 2.4.0-0 - Added check for bash loops syntax From e1c9e5d6e4fb80bd9433ab3de100db20f9ef7509 Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 12:43:43 +0300 Subject: [PATCH 08/14] Fix coverage --- check/checkers_test.go | 2 +- testdata/test_11.spec | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/check/checkers_test.go b/check/checkers_test.go index e9c1b72..5f07b3d 100644 --- a/check/checkers_test.go +++ b/check/checkers_test.go @@ -348,7 +348,7 @@ func (sc *CheckSuite) TestCheckURLForHTTPS(c *chk.C) { alerts := checkURLForHTTPS(s) - c.Assert(alerts, chk.HasLen, 1) + c.Assert(alerts, chk.HasLen, 2) c.Assert(alerts[0].Info, chk.Equals, "Domain kaos.st supports HTTPS. Replace http by https in source URL.") c.Assert(alerts[0].Line.Index, chk.Equals, 13) } diff --git a/testdata/test_11.spec b/testdata/test_11.spec index 8ae0575..c33ce82 100644 --- a/testdata/test_11.spec +++ b/testdata/test_11.spec @@ -14,8 +14,9 @@ Source0: http://kaos.st/%{name}-%{version}-1.tar.gz # perfecto:absolve Source1: http://kaos.st/%{name}-%{version}-2.tar.gz -Source2: %{name}-%{version}-3.tar.gz -Source3: http:// +Source2: http://kaos.st/%{name}-%{version}-3.tar.gz +Source3: %{name}-%{version}-3.tar.gz +Source4: http:// ################################################################################ From 0295bed9a3163d1521a5f70cc1fa9aa2c5ae17eb Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 14:39:00 +0300 Subject: [PATCH 09/14] PF-5 Add option for disabling some checks for entire spec --- README.md | 26 +++++++++- check/check.go | 26 ++++++++-- check/checkers.go | 114 ++++++++++++++++++++--------------------- check/checkers_test.go | 8 +-- check/rpmlint.go | 4 +- cli/cli.go | 10 +++- cli/render.go | 8 +-- common/perfecto.spec | 1 + 8 files changed, 123 insertions(+), 74 deletions(-) diff --git a/README.md b/README.md index f8f60a5..df8ed73 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,7 @@

-

InstallingUsing on TravisCIUsing with DockerUsageBuild StatusLicense

+

ChecksInstallingUsing on TravisCIUsing with DockerUsageBuild StatusLicense


@@ -18,6 +18,29 @@ _perfecto_ is tool for checking perfectly written RPM specs. Currently, _perfect ![Screenshot](https://gh.kaos.st/perfecto2.png) +### Checks + +* `PF1`: Checks all spec data for useless spaces; +* `PF2`: Checks all spec data for lines longer than 80 symbols; +* `PF3`: Checks release tag for using `%{?dist}` macro; +* `PF4`: Checks all scriptlets for using paths instead of macroses; +* `PF5`: Checks `install` and `clean` scriptlets for problems with `%{buildroot}` macro; +* `PF6`: Checks all scriptlets for dissimilar redirect to `/dev/null`; +* `PF7`: Checks changelog for misformatted records; +* `PF8`: Checks `build`, `install` and `check` scriptlets for using `make` which can be simplified; +* `PF9`: Checks header for required tags; +* `PF10`: Checks all spec data for unescaped percentage symbol; +* `PF11`: Checks position of `%global` and `%define` keywords; +* `PF12`: Checks length of separator comments; +* `PF13`: Checks `files` section for `%defattr`; +* `PF14`: Checks all spec data for useless binaries macroses; +* `PF15`: Checks all spec data for empty sections; +* `PF16`: Checks `files` section for indentations; +* `PF17`: Checks `setup` section options; +* `PF18`: Checks all spec data for empty lines at the end; +* `PF19`: Checks bash loops formatting; +* `PF20`: Checks sources URLs for HTTPS support. + ### Installing #### From sources @@ -141,6 +164,7 @@ Usage: perfecto {options} file… Options + --absolve, -A id… Disable some checks --format, -f format Output format (summary|tiny|short|json|xml) --lint-config, -c file Path to rpmlint configuration file --error-level, -e level Return non-zero exit code if alert level greater than given (notice|warning|error|critical) diff --git a/check/check.go b/check/check.go index 635583e..8c6c42e 100644 --- a/check/check.go +++ b/check/check.go @@ -10,6 +10,8 @@ package check import ( "sort" + "pkg.re/essentialkaos/ek.v11/sliceutil" + "github.com/essentialkaos/perfecto/spec" ) @@ -35,9 +37,10 @@ type Report struct { // Alert contain basic alert info type Alert struct { - Level uint8 `json:"level"` - Info string `json:"info"` - Line spec.Line `json:"line"` + Level uint8 `json:"level"` + Info string `json:"info"` + Line spec.Line `json:"line"` + Absolve bool `json:"absolve"` } // ////////////////////////////////////////////////////////////////////////////////// // @@ -53,6 +56,13 @@ func (s AlertSlice) Less(i, j int) bool { // ////////////////////////////////////////////////////////////////////////////////// // +// NewAlert creates new alert +func NewAlert(level uint8, info string, line spec.Line) Alert { + return Alert{level, info, line, false} +} + +// ////////////////////////////////////////////////////////////////////////////////// // + // IsPerfect return true if report doesn't have any alerts func (r *Report) IsPerfect() bool { switch { @@ -72,7 +82,7 @@ func (r *Report) IsPerfect() bool { // ////////////////////////////////////////////////////////////////////////////////// // // Check check spec -func Check(s *spec.Spec, lint bool, linterConfig string) *Report { +func Check(s *spec.Spec, lint bool, linterConfig string, absolved []string) *Report { report := &Report{} checkers := getCheckers() @@ -81,14 +91,20 @@ func Check(s *spec.Spec, lint bool, linterConfig string) *Report { appendLinterAlerts(report, alerts) } - for _, checker := range checkers { + for id, checker := range checkers { alerts := checker(s) if len(alerts) == 0 { continue } + absolve := sliceutil.Contains(absolved, id) + for _, alert := range alerts { + if absolve || alert.Line.Skip { + alert.Absolve = true + } + switch alert.Level { case LEVEL_NOTICE: report.Notices = append(report.Notices, alert) diff --git a/check/checkers.go b/check/checkers.go index dde0757..0c0a010 100644 --- a/check/checkers.go +++ b/check/checkers.go @@ -70,28 +70,28 @@ var emptyLine = spec.Line{-1, "", false} // ////////////////////////////////////////////////////////////////////////////////// // // getCheckers return slice with all supported checkers -func getCheckers() []Checker { - return []Checker{ - checkForUselessSpaces, - checkForLineLength, - checkForDist, - checkForNonMacroPaths, - checkForBuildRoot, - checkForDevNull, - checkChangelogHeaders, - checkForMakeMacro, - checkForHeaderTags, - checkForUnescapedPercent, - checkForMacroDefenitionPosition, - checkForSeparatorLength, - checkForDefAttr, - checkForUselessBinaryMacro, - checkForEmptySections, - checkForIndentInFilesSection, - checkForSetupArguments, - checkForEmptyLinesAtEnd, - checkBashLoops, - checkURLForHTTPS, +func getCheckers() map[string]Checker { + return map[string]Checker{ + "PF1": checkForUselessSpaces, + "PF2": checkForLineLength, + "PF3": checkForDist, + "PF4": checkForNonMacroPaths, + "PF5": checkForBuildRoot, + "PF6": checkForDevNull, + "PF7": checkChangelogHeaders, + "PF8": checkForMakeMacro, + "PF9": checkForHeaderTags, + "PF10": checkForUnescapedPercent, + "PF11": checkForMacroDefenitionPosition, + "PF12": checkForSeparatorLength, + "PF13": checkForDefAttr, + "PF14": checkForUselessBinaryMacro, + "PF15": checkForEmptySections, + "PF16": checkForIndentInFilesSection, + "PF17": checkForSetupOptions, + "PF18": checkForEmptyLinesAtEnd, + "PF19": checkBashLoops, + "PF20": checkURLForHTTPS, } } @@ -109,12 +109,12 @@ func checkForUselessSpaces(s *spec.Spec) []Alert { if contains(line, " ") { if strings.TrimSpace(line.Text) == "" { impLine := spec.Line{line.Index, strings.Replace(line.Text, " ", "▒", -1), line.Skip} - result = append(result, Alert{LEVEL_NOTICE, "Line contains useless spaces", impLine}) + result = append(result, NewAlert(LEVEL_NOTICE, "Line contains useless spaces", impLine)) } else if strings.TrimRight(line.Text, " ") != line.Text { cleanLine := strings.TrimRight(line.Text, " ") spaces := len(line.Text) - len(cleanLine) impLine := spec.Line{line.Index, cleanLine + strings.Repeat("▒", spaces), line.Skip} - result = append(result, Alert{LEVEL_NOTICE, "Line contains spaces at the end of line", impLine}) + result = append(result, NewAlert(LEVEL_NOTICE, "Line contains spaces at the end of line", impLine)) } } } @@ -143,7 +143,7 @@ func checkForLineLength(s *spec.Spec) []Alert { } if strutil.Len(line.Text) > 80 { - result = append(result, Alert{LEVEL_WARNING, "Line is longer than 80 symbols", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Line is longer than 80 symbols", line)) } } } @@ -167,7 +167,7 @@ func checkForDist(s *spec.Spec) []Alert { if prefix(line, "Release:") { if !contains(line, "%{?dist}") { - result = append(result, Alert{LEVEL_ERROR, "Release tag must contains %{?dist} as part of release", line}) + result = append(result, NewAlert(LEVEL_ERROR, "Release tag must contains %{?dist} as part of release", line)) } } } @@ -226,7 +226,7 @@ func checkForNonMacroPaths(s *spec.Spec) []Alert { for _, macro := range pathMacroSlice { re := regexp.MustCompile(macro.Value + `(\/|$|%)`) if re.MatchString(text) { - result = append(result, Alert{LEVEL_WARNING, fmt.Sprintf("Path \"%s\" should be used as macro \"%s\"", macro.Value, macro.Name), line}) + result = append(result, NewAlert(LEVEL_WARNING, fmt.Sprintf("Path \"%s\" should be used as macro \"%s\"", macro.Value, macro.Name), line)) } } } @@ -255,11 +255,11 @@ func checkForBuildRoot(s *spec.Spec) []Alert { } if contains(line, "$RPM_BUILD_ROOT") { - result = append(result, Alert{LEVEL_ERROR, "Build root path must be used as macro %{buildroot}", line}) + result = append(result, NewAlert(LEVEL_ERROR, "Build root path must be used as macro %{buildroot}", line)) } if contains(line, "%{buildroot}/%{_") { - result = append(result, Alert{LEVEL_WARNING, "Slash after %{buildroot} macro is useless", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Slash after %{buildroot} macro is useless", line)) } } } @@ -304,12 +304,12 @@ func checkForDevNull(s *spec.Spec) []Alert { for _, line := range section.Data { for _, v := range variations { if strings.Contains(strutil.Exclude(line.Text, " "), strutil.Exclude(v, " ")) { - result = append(result, Alert{LEVEL_NOTICE, fmt.Sprintf("Use \"&>/dev/null || :\" instead of \"%s || :\"", v), line}) + result = append(result, NewAlert(LEVEL_NOTICE, fmt.Sprintf("Use \"&>/dev/null || :\" instead of \"%s || :\"", v), line)) } } if contains(line, "|| exit 0") { - result = append(result, Alert{LEVEL_NOTICE, "Use \" || :\" instead of \" || exit 0\"", line}) + result = append(result, NewAlert(LEVEL_NOTICE, "Use \" || :\" instead of \" || exit 0\"", line)) } } } @@ -337,11 +337,11 @@ func checkChangelogHeaders(s *spec.Spec) []Alert { } if !contains(line, " - ") { - result = append(result, Alert{LEVEL_WARNING, "Misformatted changelog record header", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Misformatted changelog record header", line)) } else { separator := strings.Index(line.Text, " - ") if !strings.Contains(strutil.Substr(line.Text, separator+3, 999999), "-") { - result = append(result, Alert{LEVEL_WARNING, "Changelog record header must contain release", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Changelog record header must contain release", line)) } } } @@ -375,19 +375,19 @@ func checkForMakeMacro(s *spec.Spec) []Alert { } if prefix(line, "make") { - result = append(result, Alert{LEVEL_WARNING, "Use %{__make} macro instead of \"make\"", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Use %{__make} macro instead of \"make\"", line)) } if section.Name == spec.SECTION_INSTALL && containsField(line, "install") && contains(line, "DESTDIR") { if prefix(line, "make") || prefix(line, "%{__make}") { - result = append(result, Alert{LEVEL_WARNING, "Use %{make_install} macro instead of \"make install\"", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Use %{make_install} macro instead of \"make install\"", line)) } } if section.Name == spec.SECTION_BUILD && !contains(line, "%{?_smp_mflags}") { if prefix(line, "make") || prefix(line, "%{__make}") { if line.Text == "make" || line.Text == "%{__make}" || containsField(line, "all") { - result = append(result, Alert{LEVEL_WARNING, "Don't forget to use %{?_smp_mflags} macro with make command", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Don't forget to use %{?_smp_mflags} macro with make command", line)) } } } @@ -408,15 +408,15 @@ func checkForHeaderTags(s *spec.Spec) []Alert { for _, header := range s.GetHeaders() { if header.Package == "" { if !containsTag(header.Data, "URL:") { - result = append(result, Alert{LEVEL_ERROR, "Main package must contain URL tag", emptyLine}) + result = append(result, NewAlert(LEVEL_ERROR, "Main package must contain URL tag", emptyLine)) } } if !containsTag(header.Data, "Group:") { if header.Package == "" { - result = append(result, Alert{LEVEL_WARNING, "Main package must contain Group tag", emptyLine}) + result = append(result, NewAlert(LEVEL_WARNING, "Main package must contain Group tag", emptyLine)) } else { - result = append(result, Alert{LEVEL_WARNING, fmt.Sprintf("Package %s must contain Group tag", header.Package), emptyLine}) + result = append(result, NewAlert(LEVEL_WARNING, fmt.Sprintf("Package %s must contain Group tag", header.Package), emptyLine)) } } } @@ -440,7 +440,7 @@ func checkForUnescapedPercent(s *spec.Spec) []Alert { for _, line := range section.Data { for _, word := range strings.Fields(line.Text) { if strings.HasPrefix(word, "%") && !strings.HasPrefix(word, "%%") { - result = append(result, Alert{LEVEL_ERROR, "Symbol % must be escaped by another % (i.e % → %%)", line}) + result = append(result, NewAlert(LEVEL_ERROR, "Symbol % must be escaped by another % (i.e % → %%)", line)) } } } @@ -471,7 +471,7 @@ func checkForMacroDefenitionPosition(s *spec.Spec) []Alert { if underDescription { if contains(line, "%global ") || contains(line, "%define ") { - result = append(result, Alert{LEVEL_WARNING, "Move %define and %global to top of your spec", line}) + result = append(result, NewAlert(LEVEL_WARNING, "Move %define and %global to top of your spec", line)) } } } @@ -489,7 +489,7 @@ func checkForSeparatorLength(s *spec.Spec) []Alert { for _, line := range s.Data { if contains(line, "#") && strings.Trim(line.Text, "#") == "" && strings.Count(line.Text, "#") != 80 { - result = append(result, Alert{LEVEL_NOTICE, "Separator must be 80 symbols long", line}) + result = append(result, NewAlert(LEVEL_NOTICE, "Separator must be 80 symbols long", line)) } } @@ -521,9 +521,9 @@ func checkForDefAttr(s *spec.Spec) []Alert { switch packageName { case "": - result = append(result, Alert{LEVEL_ERROR, "%files section must contains %defattr macro", emptyLine}) + result = append(result, NewAlert(LEVEL_ERROR, "%files section must contains %defattr macro", emptyLine)) default: - result = append(result, Alert{LEVEL_ERROR, "%files section for package " + packageName + " must contains %defattr macro", emptyLine}) + result = append(result, NewAlert(LEVEL_ERROR, "%files section for package "+packageName+" must contains %defattr macro", emptyLine)) } } @@ -541,7 +541,7 @@ func checkForUselessBinaryMacro(s *spec.Spec) []Alert { for _, line := range s.Data { for _, binary := range binariesAsMacro { if contains(line, "%{__"+binary+"}") { - result = append(result, Alert{LEVEL_NOTICE, fmt.Sprintf("Useless macro %%{__%s} used for executing %s binary", binary, binary), line}) + result = append(result, NewAlert(LEVEL_NOTICE, fmt.Sprintf("Useless macro %%{__%s} used for executing %s binary", binary, binary), line)) } } } @@ -573,7 +573,7 @@ func checkForEmptySections(s *spec.Spec) []Alert { for _, section := range s.GetSections(sections...) { if len(section.Args) == 0 && isEmptyData(section.Data) { - result = append(result, Alert{LEVEL_ERROR, fmt.Sprintf("Section %%%s is empty", section.Name), s.GetLine(section.Start)}) + result = append(result, NewAlert(LEVEL_ERROR, fmt.Sprintf("Section %%%s is empty", section.Name), s.GetLine(section.Start))) } } @@ -591,7 +591,7 @@ func checkForIndentInFilesSection(s *spec.Spec) []Alert { for _, section := range s.GetSections(spec.SECTION_FILES) { for _, line := range section.Data { if strings.HasPrefix(line.Text, " ") || strings.HasPrefix(line.Text, "\t") { - result = append(result, Alert{LEVEL_NOTICE, "Don't use indent in %files section", line}) + result = append(result, NewAlert(LEVEL_NOTICE, "Don't use indent in %files section", line)) } } } @@ -599,8 +599,8 @@ func checkForIndentInFilesSection(s *spec.Spec) []Alert { return result } -// checkForSetupArguments checks setup arguments -func checkForSetupArguments(s *spec.Spec) []Alert { +// checkForSetupOptions checks setup arguments +func checkForSetupOptions(s *spec.Spec) []Alert { if len(s.Data) == 0 { return nil } @@ -610,11 +610,11 @@ func checkForSetupArguments(s *spec.Spec) []Alert { for _, section := range s.GetSections(spec.SECTION_SETUP) { switch { case containsArgs(section, "-q", "-c", "-n"): - result = append(result, Alert{LEVEL_NOTICE, "Arguments \"-q -c -n\" can be simplified to \"-qcn\"", s.GetLine(section.Start)}) + result = append(result, NewAlert(LEVEL_NOTICE, "Options \"-q -c -n\" can be simplified to \"-qcn\"", s.GetLine(section.Start))) case containsArgs(section, "-q", "-n"): - result = append(result, Alert{LEVEL_NOTICE, "Arguments \"-q -n\" can be simplified to \"-qn\"", s.GetLine(section.Start)}) + result = append(result, NewAlert(LEVEL_NOTICE, "Options \"-q -n\" can be simplified to \"-qn\"", s.GetLine(section.Start))) case containsArgs(section, "-c", "-n"): - result = append(result, Alert{LEVEL_NOTICE, "Arguments \"-c -n\" can be simplified to \"-cn\"", s.GetLine(section.Start)}) + result = append(result, NewAlert(LEVEL_NOTICE, "Options \"-c -n\" can be simplified to \"-cn\"", s.GetLine(section.Start))) } } @@ -631,7 +631,7 @@ func checkForEmptyLinesAtEnd(s *spec.Spec) []Alert { lastLine := s.Data[totalLines-1] if lastLine.Text != "" { - return []Alert{Alert{LEVEL_NOTICE, "Spec file should have empty line at the end", emptyLine}} + return []Alert{NewAlert(LEVEL_NOTICE, "Spec file should have empty line at the end", emptyLine)} } emptyLines := 0 @@ -641,7 +641,7 @@ func checkForEmptyLinesAtEnd(s *spec.Spec) []Alert { emptyLines++ } else { if emptyLines > 1 { - return []Alert{Alert{LEVEL_NOTICE, "Too much empty lines at the end of the spec", emptyLine}} + return []Alert{NewAlert(LEVEL_NOTICE, "Too much empty lines at the end of the spec", emptyLine)} } break @@ -689,7 +689,7 @@ func checkBashLoops(s *spec.Spec) []Alert { nextLineText := strings.TrimLeft(nextLine.Text, "\t ") if !strings.HasSuffix(strings.Trim(nextLineText, " "), ";do") && nextLineText == "do" { - result = append(result, Alert{LEVEL_NOTICE, "Place 'do' keyword on the same line with for/while (for ... ; do)", line}) + result = append(result, NewAlert(LEVEL_NOTICE, "Place 'do' keyword on the same line with for/while (for ... ; do)", line)) } } } @@ -726,11 +726,11 @@ func checkURLForHTTPS(s *spec.Spec) []Alert { } if isHostSupportsHTTPS(sourceDomain) { - result = append(result, Alert{ + result = append(result, NewAlert( LEVEL_WARNING, fmt.Sprintf("Domain %s supports HTTPS. Replace http by https in source URL.", sourceDomain), line, - }) + )) } } diff --git a/check/checkers_test.go b/check/checkers_test.go index 5f07b3d..30a7876 100644 --- a/check/checkers_test.go +++ b/check/checkers_test.go @@ -272,7 +272,7 @@ func (sc *CheckSuite) TestCheckForSetupArguments(c *chk.C) { c.Assert(err, chk.IsNil) c.Assert(s, chk.NotNil) - alerts := checkForSetupArguments(s) + alerts := checkForSetupOptions(s) c.Assert(alerts, chk.HasLen, 1) c.Assert(alerts[0].Info, chk.Equals, "Arguments \"-q -c -n\" can be simplified to \"-qcn\"") @@ -283,7 +283,7 @@ func (sc *CheckSuite) TestCheckForSetupArguments(c *chk.C) { c.Assert(err, chk.IsNil) c.Assert(s, chk.NotNil) - alerts = checkForSetupArguments(s) + alerts = checkForSetupOptions(s) c.Assert(alerts, chk.HasLen, 1) c.Assert(alerts[0].Info, chk.Equals, "Arguments \"-c -n\" can be simplified to \"-cn\"") @@ -294,7 +294,7 @@ func (sc *CheckSuite) TestCheckForSetupArguments(c *chk.C) { c.Assert(err, chk.IsNil) c.Assert(s, chk.NotNil) - alerts = checkForSetupArguments(s) + alerts = checkForSetupOptions(s) c.Assert(alerts, chk.HasLen, 1) c.Assert(alerts[0].Info, chk.Equals, "Arguments \"-q -n\" can be simplified to \"-qn\"") @@ -372,7 +372,7 @@ func (sc *CheckSuite) TestWithEmptyData(c *chk.C) { c.Assert(checkForUselessBinaryMacro(s), chk.IsNil) c.Assert(checkForEmptySections(s), chk.IsNil) c.Assert(checkForIndentInFilesSection(s), chk.IsNil) - c.Assert(checkForSetupArguments(s), chk.IsNil) + c.Assert(checkForSetupOptions(s), chk.IsNil) c.Assert(checkForEmptyLinesAtEnd(s), chk.IsNil) c.Assert(checkBashLoops(s), chk.IsNil) c.Assert(checkURLForHTTPS(s), chk.IsNil) diff --git a/check/rpmlint.go b/check/rpmlint.go index 392d84c..d4dcce2 100644 --- a/check/rpmlint.go +++ b/check/rpmlint.go @@ -74,9 +74,9 @@ func parseAlertLine(text string, s *spec.Spec) (Alert, bool) { switch level { case "W": - return Alert{LEVEL_ERROR, desc, s.GetLine(line)}, true + return NewAlert(LEVEL_ERROR, desc, s.GetLine(line)), true case "E": - return Alert{LEVEL_CRITICAL, desc, s.GetLine(line)}, true + return NewAlert(LEVEL_CRITICAL, desc, s.GetLine(line)), true } return Alert{}, false diff --git a/cli/cli.go b/cli/cli.go index 32b19e6..209c0f1 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -10,6 +10,7 @@ package cli import ( "fmt" "os" + "strings" "pkg.re/essentialkaos/ek.v11/env" "pkg.re/essentialkaos/ek.v11/fmtc" @@ -41,6 +42,7 @@ const ( OPT_FORMAT = "f:format" OPT_LINT_CONFIG = "c:lint-config" OPT_ERROR_LEVEL = "e:error-level" + OPT_ABSOLVE = "A:absolve" OPT_QUIET = "q:quiet" OPT_NO_LINT = "nl:no-lint" OPT_NO_COLOR = "nc:no-color" @@ -72,6 +74,7 @@ const ( // options map var optMap = options.Map{ + OPT_ABSOLVE: {Mergeble: true}, OPT_FORMAT: {Type: options.STRING}, OPT_LINT_CONFIG: {Type: options.STRING}, OPT_ERROR_LEVEL: {Type: options.STRING}, @@ -170,7 +173,11 @@ func checkSpec(file, format string) int { return 1 } - report := check.Check(s, !options.GetB(OPT_NO_LINT), options.GetS(OPT_LINT_CONFIG)) + report := check.Check( + s, !options.GetB(OPT_NO_LINT), + options.GetS(OPT_LINT_CONFIG), + strings.Split(options.GetS(OPT_ABSOLVE), ","), + ) if report.IsPerfect() { if !options.GetB(OPT_QUIET) { @@ -286,6 +293,7 @@ func showUsage() { func genUsage() *usage.Info { info := usage.NewInfo("", "file…") + info.AddOption(OPT_ABSOLVE, "Disable some checks by their ID", "id…") info.AddOption(OPT_FORMAT, "Output format {s-}(summary|tiny|short|json|xml){!}", "format") info.AddOption(OPT_LINT_CONFIG, "Path to rpmlint configuration file", "file") info.AddOption(OPT_ERROR_LEVEL, "Return non-zero exit code if alert level greater than given {s-}(notice|warning|error|critical){!}", "level") diff --git a/cli/render.go b/cli/render.go index 2c535fe..5aaca9c 100644 --- a/cli/render.go +++ b/cli/render.go @@ -182,13 +182,13 @@ func renderTinyReport(s *spec.Spec, r *check.Report) { for _, alert := range alerts { if options.GetB(OPT_NO_COLOR) { - if alert.Line.Skip { + if alert.Absolve { fmtc.Printf("X ") } else { fmtc.Printf(fallbackLevel[level] + " ") } } else { - if alert.Line.Skip { + if alert.Absolve { fmtc.Printf("{s-}%s{!}", "•") } else { fmtc.Printf(fgColor[level]+"%s{!}", "•") @@ -241,7 +241,7 @@ func renderAlert(alert check.Alert) { fmtc.Printf(fg + "│ {!}") if alert.Line.Index != -1 { - if alert.Line.Skip { + if alert.Absolve { fmtc.Printf(hl+"[%d]{!} {s}[A]{!} "+fg+"%s{!}\n", alert.Line.Index, alert.Info) } else { fmtc.Printf(hl+"[%d]{!} "+fg+"%s{!}\n", alert.Line.Index, alert.Info) @@ -266,7 +266,7 @@ func renderShortAlert(alert check.Alert) { } if alert.Line.Index != -1 { - if alert.Line.Skip { + if alert.Absolve { fmtc.Printf(hl+"[%d]{!} {s}[A]{!} "+fg+"%s{!}\n", alert.Line.Index, alert.Info) } else { fmtc.Printf(hl+"[%d]{!} "+fg+"%s{!}\n", alert.Line.Index, alert.Info) diff --git a/common/perfecto.spec b/common/perfecto.spec index a63c3fb..bed4bd3 100644 --- a/common/perfecto.spec +++ b/common/perfecto.spec @@ -88,6 +88,7 @@ fi %changelog * Fri Oct 25 2019 Anton Novojilov - 2.5.0-0 +- Added option for disabling some checks for entire spec - Added check for HTTPS support on a source domain - Added cache for HTTPS checks - ek package updated to the latest version From f8fafe9e92a9f0df22c8ea9080631bcec59cb693 Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 15:04:31 +0300 Subject: [PATCH 10/14] Fix tests --- check/checkers_test.go | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/check/checkers_test.go b/check/checkers_test.go index 30a7876..dab41ea 100644 --- a/check/checkers_test.go +++ b/check/checkers_test.go @@ -275,7 +275,7 @@ func (sc *CheckSuite) TestCheckForSetupArguments(c *chk.C) { alerts := checkForSetupOptions(s) c.Assert(alerts, chk.HasLen, 1) - c.Assert(alerts[0].Info, chk.Equals, "Arguments \"-q -c -n\" can be simplified to \"-qcn\"") + c.Assert(alerts[0].Info, chk.Equals, "Options \"-q -c -n\" can be simplified to \"-qcn\"") c.Assert(alerts[0].Line.Index, chk.Equals, 33) s, err = spec.Read("../testdata/test_5.spec") @@ -286,7 +286,7 @@ func (sc *CheckSuite) TestCheckForSetupArguments(c *chk.C) { alerts = checkForSetupOptions(s) c.Assert(alerts, chk.HasLen, 1) - c.Assert(alerts[0].Info, chk.Equals, "Arguments \"-c -n\" can be simplified to \"-cn\"") + c.Assert(alerts[0].Info, chk.Equals, "Options \"-c -n\" can be simplified to \"-cn\"") c.Assert(alerts[0].Line.Index, chk.Equals, 41) s, err = spec.Read("../testdata/test_6.spec") @@ -297,7 +297,7 @@ func (sc *CheckSuite) TestCheckForSetupArguments(c *chk.C) { alerts = checkForSetupOptions(s) c.Assert(alerts, chk.HasLen, 1) - c.Assert(alerts[0].Info, chk.Equals, "Arguments \"-q -n\" can be simplified to \"-qn\"") + c.Assert(alerts[0].Info, chk.Equals, "Options \"-q -n\" can be simplified to \"-qn\"") c.Assert(alerts[0].Line.Index, chk.Equals, 31) } @@ -384,7 +384,7 @@ func (sc *CheckSuite) TestRPMLint(c *chk.C) { c.Assert(s, chk.NotNil) c.Assert(err, chk.IsNil) - r := Check(s, true, "") + r := Check(s, true, "", nil) c.Assert(r, chk.NotNil) c.Assert(r.IsPerfect(), chk.Equals, true) @@ -394,11 +394,22 @@ func (sc *CheckSuite) TestRPMLint(c *chk.C) { c.Assert(s, chk.NotNil) c.Assert(err, chk.IsNil) - r = Check(s, true, "") + r = Check(s, true, "", nil) c.Assert(r, chk.NotNil) c.Assert(r.IsPerfect(), chk.Equals, false) + s, err = spec.Read("../testdata/test_11.spec") + + c.Assert(s, chk.NotNil) + c.Assert(err, chk.IsNil) + + r = Check(s, true, "", []string{"PF20"}) + + c.Assert(r, chk.NotNil) + c.Assert(r.Warnings, chk.HasLen, 2) + c.Assert(r.Warnings[0].Absolve, chk.Equals, true) + rpmLintBin = "echo" s = &spec.Spec{File: ""} c.Assert(Lint(s, ""), chk.IsNil) From 62f82facf6730a15fb168fc9842eaa0c435a43ae Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 15:20:59 +0300 Subject: [PATCH 11/14] Fix usage in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index df8ed73..bba94e9 100644 --- a/README.md +++ b/README.md @@ -164,7 +164,7 @@ Usage: perfecto {options} file… Options - --absolve, -A id… Disable some checks + --absolve, -A id… Disable some checks by their ID --format, -f format Output format (summary|tiny|short|json|xml) --lint-config, -c file Path to rpmlint configuration file --error-level, -e level Return non-zero exit code if alert level greater than given (notice|warning|error|critical) From 449edd93a8e44b019625dddb7f5840c0a00a2d5a Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 15:25:31 +0300 Subject: [PATCH 12/14] Improve XML encoder --- cli/render_xml.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/render_xml.go b/cli/render_xml.go index 7e279d9..95f231d 100644 --- a/cli/render_xml.go +++ b/cli/render_xml.go @@ -45,7 +45,7 @@ func renderAlertsAsXML(category string, alerts []check.Alert) { fmt.Printf(" <%s>\n", category) for _, alert := range alerts { - fmt.Println(" ") + fmt.Printf(" \n", alert.Absolve) fmt.Printf(" %s\n", escapeStringForXML(alert.Info)) if alert.Line.Index != -1 { From 8e32632fb2ad99bc82ff1db26d4cdb7a7771a36d Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 15:56:54 +0300 Subject: [PATCH 13/14] Minor cache time fix --- check/checkers.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check/checkers.go b/check/checkers.go index 0c0a010..e494599 100644 --- a/check/checkers.go +++ b/check/checkers.go @@ -704,7 +704,7 @@ func checkURLForHTTPS(s *spec.Spec) []Alert { } if httpCheckCache == nil { - httpCheckCache = cache.New(time.Minute, 0) + httpCheckCache = cache.New(time.Hour, 0) } sources := s.GetSources() From 4751b41ad903c5dd5679652a8688a41083612874 Mon Sep 17 00:00:00 2001 From: Anton Novojilov Date: Fri, 25 Oct 2019 18:57:27 +0300 Subject: [PATCH 14/14] Improve README --- README.md | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index bba94e9..73ebcce 100644 --- a/README.md +++ b/README.md @@ -20,26 +20,26 @@ _perfecto_ is tool for checking perfectly written RPM specs. Currently, _perfect ### Checks -* `PF1`: Checks all spec data for useless spaces; -* `PF2`: Checks all spec data for lines longer than 80 symbols; -* `PF3`: Checks release tag for using `%{?dist}` macro; -* `PF4`: Checks all scriptlets for using paths instead of macroses; -* `PF5`: Checks `install` and `clean` scriptlets for problems with `%{buildroot}` macro; -* `PF6`: Checks all scriptlets for dissimilar redirect to `/dev/null`; -* `PF7`: Checks changelog for misformatted records; -* `PF8`: Checks `build`, `install` and `check` scriptlets for using `make` which can be simplified; -* `PF9`: Checks header for required tags; -* `PF10`: Checks all spec data for unescaped percentage symbol; -* `PF11`: Checks position of `%global` and `%define` keywords; -* `PF12`: Checks length of separator comments; -* `PF13`: Checks `files` section for `%defattr`; -* `PF14`: Checks all spec data for useless binaries macroses; -* `PF15`: Checks all spec data for empty sections; -* `PF16`: Checks `files` section for indentations; -* `PF17`: Checks `setup` section options; -* `PF18`: Checks all spec data for empty lines at the end; -* `PF19`: Checks bash loops formatting; -* `PF20`: Checks sources URLs for HTTPS support. +* `PF1` Checks all spec data for useless spaces; +* `PF2` Checks all spec data for lines longer than 80 symbols; +* `PF3` Checks release tag for using `%{?dist}` macro; +* `PF4` Checks all scriptlets for using paths instead of macroses; +* `PF5` Checks `install` and `clean` scriptlets for problems with `%{buildroot}` macro; +* `PF6` Checks all scriptlets for dissimilar redirect to `/dev/null`; +* `PF7` Checks changelog for misformatted records; +* `PF8` Checks `build`, `install` and `check` scriptlets for using `make` which can be simplified; +* `PF9` Checks header for required tags; +* `PF10` Checks all spec data for unescaped percentage symbol; +* `PF11` Checks position of `%global` and `%define` keywords; +* `PF12` Checks length of separator comments; +* `PF13` Checks `files` section for `%defattr`; +* `PF14` Checks all spec data for useless binaries macroses; +* `PF15` Checks all spec data for empty sections; +* `PF16` Checks `files` section for indentations; +* `PF17` Checks `setup` section options; +* `PF18` Checks all spec data for empty lines at the end; +* `PF19` Checks bash loops formatting; +* `PF20` Checks sources URLs for HTTPS support. ### Installing