Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add gpg key id. #13

Merged
merged 1 commit into from
Nov 29, 2023
Merged

Add gpg key id. #13

merged 1 commit into from
Nov 29, 2023

Conversation

pstankie
Copy link
Contributor

EF issue: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/3684#note_1433269

Signed-off-by: Pawel Stankiewicz <pawel.stankiewicz@eclipse-foundation.org>
@pstankie pstankie requested review from a team as code owners November 28, 2023 14:35
Copy link

Diff for d2c5315:
Printing local diff for configuration at '/home/runner/work/.eclipsefdn/.eclipsefdn/otterdog-configs/otterdog.json'

Actions are indicated with the following symbols:
+   create
!   modify
!   forced update
-   delete

Organization eclipse-kuksa[id=eclipse-kuksa]

+   add org_secret[name="ORG_GPG_KEY_ID"] {
+     name                                                     = "ORG_GPG_KEY_ID"
+     selected_repositories                                    = []
+     visibility                                               = "public"
+   }
  
  Plan: 1 to add, 0 to change, 0 to delete.
Showing diff to a canonical version of the configuration at '/home/runner/work/.eclipsefdn/.eclipsefdn/otterdog-configs/otterdog.json'

Organization eclipse-kuksa[id=eclipse-kuksa]
--- original
+++ canonical
@@ -1,17 +1,22 @@
 local orgs = import 'vendor/otterdog-defaults/otterdog-defaults.libsonnet';
 
+
+local kuksa_default_branch_protection_rule(pattern) =
+  orgs.newBranchProtectionRule(pattern) {
+
+
+        dismisses_stale_reviews: true
+        require_last_push_approval: true
+        required_approving_review_count: 1
+        requires_strict_status_checks: true
+  };
 orgs.newOrg('eclipse-kuksa') {
   _repositories+:: [
     orgs.newRepo('kuksa-actions') {
       allow_merge_commit: true
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       delete_branch_on_merge: false
       dependabot_security_updates_enabled: true
@@ -26,12 +31,7 @@
       allow_squash_merge: false
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       dependabot_security_updates_enabled: true
       web_commit_signoff_required: false
@@ -45,12 +45,7 @@
       allow_squash_merge: false
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       dependabot_security_updates_enabled: true
       web_commit_signoff_required: false
@@ -62,12 +57,7 @@
       allow_merge_commit: true
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       delete_branch_on_merge: false
       dependabot_security_updates_enabled: true
@@ -90,12 +80,7 @@
       allow_merge_commit: true
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       delete_branch_on_merge: false
       dependabot_alerts_enabled: false
@@ -110,12 +95,7 @@
       allow_merge_commit: true
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       delete_branch_on_merge: false
       dependabot_security_updates_enabled: true
@@ -128,12 +108,7 @@
       allow_merge_commit: true
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('main') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('main')
       ]
       delete_branch_on_merge: false
       dependabot_security_updates_enabled: true
@@ -146,12 +121,7 @@
       allow_merge_commit: true
       allow_update_branch: false
       branch_protection_rules: [
-        orgs.newBranchProtectionRule('master') {
-          dismisses_stale_reviews: true
-          require_last_push_approval: true
-          required_approving_review_count: 1
-          requires_strict_status_checks: true
-        }
+        kuksa_default_branch_protection_rule('master')
       ]
       default_branch: "master"
       delete_branch_on_merge: false

@pstankie pstankie assigned pstankie and unassigned pstankie Nov 28, 2023
Copy link
Contributor

@wba2hi wba2hi left a comment

Choose a reason for hiding this comment

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

LGTM

Additional Notes: (maybe obvious, but for me as somebody with no experience with GPG it wasn't)

  • KEY_ID must be of the corresponding signing subkey
  • KEY_ID must be in short form (last 8 chars of key)

@pstankie
Copy link
Contributor Author

Actually, key id length is 17 https://keyserver.ubuntu.com/pks/lookup?search=059BD97D6B860B08&fingerprint=on&op=index
If that doesn't work we can setup variable (?)

@wba2hi
Copy link
Contributor

wba2hi commented Nov 29, 2023

Actually, key id length is 17 https://keyserver.ubuntu.com/pks/lookup?search=059BD97D6B860B08&fingerprint=on&op=index If that doesn't work we can setup variable (?)

Sorry, that maybe was a bit confusing. The key id length itself does not matter.

It only matters that the secret which is exposed here uses the short form, so the last 8 chars :) When I tried it with the full length key id the gradle build failed because it explicitely wanted to have the short version.

See the build here for comparison: https://pipelinesghubeus6.actions.githubusercontent.com/jHbhpd0jty1Hpof85CEptK1oO87frIUJ1QK31h200mlfaxtYTh/_apis/pipelines/1/runs/42/signedlogcontent/2?urlExpires=2023-11-29T08%3A31%3A37.0240824Z&urlSigningMethod=HMACV1&urlSignature=Ckjr8wtfOiXgbn%2FSSfUHUpKS%2FQyWgS9vtRu7HJtbbXc%3D

It shows the following line:

2023-11-28T12:33:58.2378175Z 2023-11-28T12:33:58.116+0000 [ERROR] [org.gradle.internal.buildevents.BuildExceptionReporter] Caused by: java.lang.IllegalStateException: The key ID must be in a valid form (eg 00B5050F or 0x00B5050F), given value: ***

Working link: https://github.com/boschglobal/kuksa-android-sdk/actions/runs/7018773710/job/19094859366

Edit: We probably also can simply truncate it from our side, so maybe not that important :)

@eclipsewebmaster
Copy link

Yes, I would suggest you truncate the key. The actual key_id, generated by gpg2 has the length stored in our vault and corresponds to key id stored in keyserver and changing that on our side would be inconsistent and insecure.

@pstankie pstankie merged commit 2d4b45f into main Nov 29, 2023
2 checks passed
@pstankie pstankie deleted the add-gpg-keyid branch November 29, 2023 10:53
@pstankie
Copy link
Contributor Author

changes are live

1 similar comment
@pstankie
Copy link
Contributor Author

changes are live

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants