added support to sign linux artifacts #1352

abhinavGupta16 · 2021-12-16T01:45:15Z

Signed-off-by: Abhinav Gupta abhng@amazon.com

Description

Automate linux signing to sign any artifact or all artifacts in a directory

Issues Resolved

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

codecov-commenter · 2021-12-16T02:11:31Z

Codecov Report

Merging #1352 (eed9b7f) into main (ea5a964) will increase coverage by 0.06%.
The diff coverage is 97.05%.

@@             Coverage Diff              @@
##               main    #1352      +/-   ##
============================================
+ Coverage     94.02%   94.08%   +0.06%     
  Complexity       11       11              
============================================
  Files           136      139       +3     
  Lines          3011     3061      +50     
  Branches          8        8              
============================================
+ Hits           2831     2880      +49     
- Misses          172      173       +1     
  Partials          8        8

Impacted Files	Coverage Δ
src/sign_workflow/signer.py	`94.11% <90.00%> (-2.66%)`	⬇️
src/sign_workflow/sign_artifacts.py	`98.03% <98.03%> (ø)`
src/run_sign.py	`95.23% <100.00%> (+2.64%)`	⬆️
tests/jenkins/jobs/PromoteArtifacts_Jenkinsfile	`100.00% <0.00%> (ø)`
.../jenkins/jobs/PromoteArtifacts_actions_Jenkinsfile	`100.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea5a964...eed9b7f. Read the comment docs.

src/sign_workflow/sign_existing_artifacts.py

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

src/sign_workflow/sign_existing_artifacts.py

src/run_sign.py

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

src/run_sign.py

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

src/sign_workflow/sign_existing_artifacts.py

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

tests/test_run_sign.py

vars/signArtifacts.groovy

dblock

Close!

dblock · 2021-12-17T15:32:36Z

src/run_sign.py

 from system import console

+ACCEPTED_SIGNATURE_FILE_TYPES = [".sig", ".asc"]


Isn't this already Signer.ACCEPTED_FILE_TYPES? Use that.

No, this is accepted Signature types, different from accepted file types

src/run_sign.py

src/sign_workflow/sign_artifacts.py

dblock · 2021-12-17T15:37:50Z

src/sign_workflow/sign_artifacts.py

+
+    def __sign__(self):
+        artifacts = [self.target.name]
+        basename = self.target.parent


These variables aren't reused, just pass the arguments as is below.

Add __sign_artifact__ that takes one artifact.

Variables just make it clear as to what is being passed to the function. I think we can leave it like this

src/sign_workflow/signer.py

src/sign_workflow/sign_artifacts.py

tests/tests_sign_workflow/test_sign_artifacts.py

dblock · 2021-12-17T15:42:48Z

tests/tests_sign_workflow/test_sign_artifacts.py

+
+        self.assertEqual(type(SignWithManifest), type(klass))
+
+        path = Path(r"/dummy/path/")


Split up in separate tests.

I think it should be one method, since it just verifies multiple if conditions with no workflow. It would just increase verbosity and complexity. This is clear too.

It's not a must have, but my reasoning is that these tests can fail independently, and methods are cheap.

dblock · 2021-12-17T15:43:02Z

tests/tests_sign_workflow/test_sign_artifacts.py

+        artifact_type = 'dummy'
+        sigtype = '.asc'
+
+        klass = SignArtifacts.from_path(path, component, artifact_type, sigtype)


Same as above, split up into individual tests for each type.

Same reason as before.

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

tests/tests_sign_workflow/test_sign_artifacts.py

dblock · 2021-12-18T14:10:39Z

tests/tests_sign_workflow/test_sign_artifacts.py

+
+        self.assertEqual(type(SignWithManifest), type(klass))
+
+        path = Path(r"/dummy/path/")


It's not a must have, but my reasoning is that these tests can fail independently, and methods are cheap.

tests/tests_sign_workflow/test_sign_artifacts.py

peternied

Thanks for addressing my comments, great work enhancing this flow.

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

abhinavGupta16 · 2021-12-21T02:51:33Z

Thanks @dblock for helping out with the test cases

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

dblock

Works for me.

I would also remove all the passing around of Signer() which is always the same class, and create an instance of it at the lowest level.

dblock · 2021-12-21T19:10:40Z

src/sign_workflow/signer.py

-                "".join(pathlib.Path(file_name).suffixes),
-            ]
-            for x in Signer.ACCEPTED_FILE_TYPES
+            file_name.endswith(x) for x in Signer.ACCEPTED_FILE_TYPES


Use os.path.splitext to get the extension from the file name, then in to check ether it’s one of the accepted types.

That does not work since we have artifacts with names like opensearch-1.0.0.tar.gz. That is why I changed it to endswith

We could just change .tar.gz into .gz. Technically the extension of that file is not .tar.gz, it’s .gz.

@dblock

@dblock approved the changes on behalf of opensearch-project/engineering-effectiveness

added support to sign linux artifacts

97cc4f4

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

abhinavGupta16 self-assigned this Dec 16, 2021

abhinavGupta16 requested review from dblock, peternied, a team and gaiksaya December 16, 2021 01:47

abhinavGupta16 added 3 commits December 15, 2021 17:58

changed testcase to accomadate changes

e543d50

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

changed testcase to accomadate changes

0c20334

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

changed testcase to accomadate changes

0040745

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

gaiksaya reviewed Dec 16, 2021

View reviewed changes

src/sign_workflow/sign_existing_artifacts.py Outdated Show resolved Hide resolved

gaiksaya reviewed Dec 16, 2021

View reviewed changes

src/sign_workflow/sign_existing_artifacts.py Outdated Show resolved Hide resolved

abhinavGupta16 added 4 commits December 15, 2021 19:15

changed testcase to accomadate changes

83fbc32

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

removed unused import

5355b12

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

removed unwanted file

5f59b78

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

added support to recursively find artifacts and sign

4735ea2

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

abhinavGupta16 requested a review from gaiksaya December 16, 2021 09:07

change opensearch-signer-url

1ebc13e

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

peternied reviewed Dec 16, 2021

View reviewed changes

src/sign_workflow/sign_existing_artifacts.py Outdated Show resolved Hide resolved

src/run_sign.py Outdated Show resolved Hide resolved

fix isort

33bd806

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

dblock requested changes Dec 16, 2021

View reviewed changes

src/run_sign.py Outdated Show resolved Hide resolved

src/run_sign.py Outdated Show resolved Hide resolved

src/run_sign.py Outdated Show resolved Hide resolved

src/run_sign.py Outdated Show resolved Hide resolved

accomodate review changes

1b1d74b

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

abhinavGupta16 requested a review from peternied December 16, 2021 16:00

dblock reviewed Dec 16, 2021

View reviewed changes

src/sign_workflow/sign_existing_artifacts.py Outdated Show resolved Hide resolved

refactored code and added test cases

67f1e5e

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

peterzhuamazon mentioned this pull request Dec 17, 2021

Add sha512 generation to promotion job #1367

Merged

1 task

abhinavGupta16 added 2 commits December 16, 2021 23:16

added test cases

25747a8

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

added signArtifacts groovy file

ea965b1

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

peternied suggested changes Dec 17, 2021

View reviewed changes

dblock previously requested changes Dec 17, 2021

View reviewed changes

abhinavGupta16 added 2 commits December 18, 2021 03:21

code refactor

7a16904

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

code refactor

5004413

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

dblock reviewed Dec 18, 2021

View reviewed changes

peternied approved these changes Dec 18, 2021

View reviewed changes

abhinavGupta16 added 2 commits December 18, 2021 23:15

updated test case

a641a21

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

added changes to test case

9091fdc

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

abhinavGupta16 linked an issue Dec 19, 2021 that may be closed by this pull request

upgrade Signer.py to be able to sign artifacts with .sig in a directory #1382

Closed

5 tasks

abhinavGupta16 added 14 commits December 20, 2021 11:29

added mock git repo to test

4543a94

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

removing mock git repo to test

a9ac127

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

removing mock git repo to test and changing mock

342dfb3

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

removing mock git repo to test and changing mock

fff94cd

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

fix existing test cases added new with todo

6bf4e44

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

added data for test cases

0cccc59

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

fixed one test case

db40e12

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

fix pip8 and isort issues

9f5aa11

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

added dummy tar files

84bf99f

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

add and fix test cases

5e8d28c

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

add and fix test cases

9fabfaf

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

added data file

4520a00

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

changed filename for dummy artifact

14901d0

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

changed filename for dummy artifact

9cb944c

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

abhinavGupta16 marked this pull request as ready for review December 21, 2021 02:50

abhinavGupta16 requested a review from dblock December 21, 2021 02:51

refactored test case

eed9b7f

Signed-off-by: Abhinav Gupta <abhng@amazon.com>

dblock approved these changes Dec 21, 2021

View reviewed changes

abhinavGupta16 merged commit c18e136 into opensearch-project:main Dec 21, 2021

abhinavGupta16 deleted the abhng-sign-artifacts branch December 21, 2021 19:44

abhinavGupta16 mentioned this pull request Jan 21, 2022

Signing by ASC should be supported #1526

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added support to sign linux artifacts #1352

added support to sign linux artifacts #1352

abhinavGupta16 commented Dec 16, 2021 •

edited

Loading

codecov-commenter commented Dec 16, 2021 •

edited

Loading

dblock left a comment

dblock Dec 17, 2021

abhinavGupta16 Dec 18, 2021

dblock Dec 17, 2021

abhinavGupta16 Dec 18, 2021

dblock Dec 17, 2021

abhinavGupta16 Dec 18, 2021 •

edited

Loading

dblock Dec 18, 2021

dblock Dec 17, 2021

abhinavGupta16 Dec 18, 2021

dblock Dec 18, 2021

peternied left a comment

abhinavGupta16 commented Dec 21, 2021

dblock left a comment

dblock Dec 21, 2021

abhinavGupta16 Dec 21, 2021

dblock Dec 21, 2021

		from system import console

		ACCEPTED_SIGNATURE_FILE_TYPES = [".sig", ".asc"]


		self.assertEqual(type(SignWithManifest), type(klass))

		path = Path(r"/dummy/path/")

added support to sign linux artifacts #1352

added support to sign linux artifacts #1352

Conversation

abhinavGupta16 commented Dec 16, 2021 • edited Loading

Description

Issues Resolved

Check List

codecov-commenter commented Dec 16, 2021 • edited Loading

Codecov Report

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavGupta16 Dec 18, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternied left a comment

Choose a reason for hiding this comment

abhinavGupta16 commented Dec 21, 2021

dblock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abhinavGupta16 commented Dec 16, 2021 •

edited

Loading

codecov-commenter commented Dec 16, 2021 •

edited

Loading

abhinavGupta16 Dec 18, 2021 •

edited

Loading