Skip to content

GitLab

Merged
Created by Administrator@rootOwner

Add system hook for ssh key changes

Created by: duk3luk3

Enabling SSH Key create/delete notification in system hooks makes it possible to automatically push ssh keys to other systems so gitlab can be more tightly integrated with a development environment.

Code is self-explaining. It adds after_create and after_destroy hooks to the Key model and adds logic to process a Key update in the build_event_data method of the system hook service.

Request to merge github/fork/duk3luk3/system-hook-key-feature into master
Checking approval status

Merged by (Jun 20, 2025 8:59am UTC)

The changes were merged into master with 40fc4261

The source branch has been deleted

  • Administrator
    Administrator @root ·
    Owner

    Created by: TeatroIO

    I've prepared a stage. Click to open.

    By Administrator on 2014-08-18T14:55:54 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/services/system_hooks_service.rb
    22 22 }
    23 23
    24 24 case model
    25 when Key
    26 data.merge!(
    27 key: model.key,
    28 id: model.id
    29 )
  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/services/system_hooks_service.rb
    22 22 }
    23 23
    24 24 case model
    25 when Key
    26 data.merge!(
    27 key: model.key,
    28 id: model.id
  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/services/system_hooks_service.rb
    22 22 }
    23 23
    24 24 case model
    25 when Key
    26 data.merge!(
    27 key: model.key,
    28 id: model.id
    29 )
    30 if model.user
    • Administrator
      Administrator @root ·
      Owner

      Created by: houndci-bot

      Indent the right brace the same as the first position after the preceding left parenthesis.

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/services/system_hooks_service.rb
    22 22 }
    23 23
    24 24 case model
    25 when Key
    26 data.merge!(
    27 key: model.key,
    • Administrator
      Administrator @root ·
      Owner

      Created by: houndci-bot

      Use 2 spaces for indentation in a hash, relative to the first position after the preceding left parenthesis.
      Tab detected.

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/services/system_hooks_service.rb
    22 22 }
    23 23
    24 24 case model
    25 when Key
    26 data.merge!(
    • Administrator
      Administrator @root ·
      Owner

      Created by: houndci-bot

      Redundant curly braces around a hash parameter.

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Corresponding feedback post is here: http://feedback.gitlab.com/forums/176466-general/suggestions/6311549-add-ssh-keys-to-system-hooks

    By Administrator on 2014-08-18T14:59:16 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Whoops. Fixed the tabs.

    By Administrator on 2014-08-18T15:11:57 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    spec/services/system_hooks_service_spec.rb
    21 24 it { event_name(project, :destroy).should eq "project_destroy" }
    22 25 it { event_name(users_project, :create).should eq "user_add_to_team" }
    23 26 it { event_name(users_project, :destroy).should eq "user_remove_from_team" }
    27 it { event_name(key, :create).should eq 'key_create' }
    • Administrator
      Administrator @root ·
      Owner

      Created by: houndci-bot

      Prefer single-quoted strings when you don't need string interpolation or special symbols.

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    spec/services/system_hooks_service_spec.rb
    21 24 it { event_name(project, :destroy).should eq "project_destroy" }
    22 25 it { event_name(users_project, :create).should eq "user_add_to_team" }
    23 26 it { event_name(users_project, :destroy).should eq "user_remove_from_team" }
    27 it { event_name(key, :create).should eq 'key_create' }
    28 it { event_name(key, :destroy).should eq 'key_destroy' }
    • Administrator
      Administrator @root ·
      Owner

      Created by: houndci-bot

      Prefer single-quoted strings when you don't need string interpolation or special symbols.

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    I've now also added tests.

    By Administrator on 2014-08-18T18:36:21 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    :thumbsup: Could you also add documentation?

    By Administrator on 2014-08-18T18:36:51 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Of course. How are the "event names" in doc/system_hooks/system_hooks.md that are listed in the first line constructed? I can't see where they come from exactly...

    By Administrator on 2014-08-18T18:44:09 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    I went ahead and made up something that followed the convention of being similar but not exactly identical to the actual event names ;-)

    By Administrator on 2014-08-18T18:54:30 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    I think this are the event names like on the testcases https://github.com/duk3luk3/gitlabhq/blob/system-hook-key-feature/spec/services/system_hooks_service_spec.rb#L20 However, they are not consistent with the documentation. Since you update, could you fix the other ones?

    By Administrator on 2014-08-18T18:55:29 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    It's user_create, user_destroy, project_create, project_destroy, user_add_to_team, user_remove_from_team

    And yours: key_create, key_destroy

    By Administrator on 2014-08-18T18:58:00 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Done :-)

    By Administrator on 2014-08-18T18:59:37 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    :thumbsup: Could you fix the hound warnings, and add a changelog entry? But for 7.3 since it will not make it into 7.2 (already feature freeze).

    By Administrator on 2014-08-18T19:01:32 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    I fixed all the hound warnings that actually showed a discrepancy to the coding style of the files I was editing. If I fix them all, I also have to fix up the rest of the file, and I'll be touching the entire file and not just where I actually added/changed program logic. Should I put the style fixups into a separate commit to make it easier to see the program logic changes?

    By Administrator on 2014-08-18T19:08:26 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    It's sufficient to only change the new lines.

    By Administrator on 2014-08-18T19:10:54 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    All done!

    By Administrator on 2014-08-18T19:22:29 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    Awesome :heart: @jvanbaarsen Looks good to me, can you also review?

    By Administrator on 2014-08-18T19:26:14 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/models/key.rb
    56 58 NotificationService.new.new_key(self)
    57 59 end
    58 60
    61 def post_create_hook
    62 SystemHooksService.new.execute_hooks_for(self, :create)
    • Administrator
      Administrator @root ·
      Owner

      Created by: jvanbaarsen

      What do you think about moving this into a worker? Since it might take a little while for the hook to complete. Although im not sure how other hooks are beeing used. Maybe you can check that out :)

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/models/key.rb
    56 58 NotificationService.new.new_key(self)
    57 59 end
    58 60
    61 def post_create_hook
    62 SystemHooksService.new.execute_hooks_for(self, :create)
    • Administrator
      Administrator @root ·
      Owner

      Created by: duk3luk3

      All system hooks are already being executed via a worker if I read the code right. execute_hooks_for calls execute_hooks calls async_execute_hook.

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root started a thread on commit 40fc4261
    app/models/key.rb
    56 58 NotificationService.new.new_key(self)
    57 59 end
    58 60
    61 def post_create_hook
    62 SystemHooksService.new.execute_hooks_for(self, :create)
    • Administrator
      Administrator @root ·
      Owner

      Created by: jvanbaarsen

      @duk3luk3 Good point! You are more awake then I :D haha

      By Administrator on 2014-09-02T23:27:23 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: jvanbaarsen

    @randx Looks good to merge

    By Administrator on 2014-08-18T19:38:05 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: samrocketman

    :thumbsup: Sweet. This has been a much desired feature at Drexel. (I'm formerly github user sag47 though still sag47 in IRC)

    By Administrator on 2014-08-18T22:46:18 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: Razer6

    There are failing tests, relating to keys says TravisCI. Can check them?

    By Administrator on 2014-08-19T11:47:36 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    I'm looking at it.

    By Administrator on 2014-08-19T17:05:53 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Could it be that the key-related tests are failing because of

    Warning You are running as user travis, we hope you know what you are doing. Things may work/fail for the wrong reasons. For correct results you should run this as user git.

    ?

    There are errors like this happening:

    ................................../home/travis/build/gitlabhq/gitlabhq/tmp/tests/gitlab-shell/lib/gitlab_keys.rb:87:in `initialize': No such file or directory - /home/git/.ssh/authorized_keys.lock (Errno::ENOENT)

    That seems to be a problem with the test configuration.

    My code is failing because it expects keys that belong to a user but is called with key instances where user is nil. It seems that that is valid so I'll change it in my code.

    By Administrator on 2014-08-19T17:46:25 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Done. Hopefully that fixes all the test failures.

    EDIT: Ran most of the test suite and tests that were failing pass now.

    By Administrator on 2014-08-19T20:11:50 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    I merged in master again. This should be ready to merge unless something got screwed up.

    By Administrator on 2014-09-02T23:29:33 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: dblessing

    Test failures now seem to be related to RubyGem timeouts and other weird unrelated errors. I think this is OK, but it's hard to tell. @randx Can you take a look, please?

    By Administrator on 2014-09-03T10:01:36 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: duk3luk3

    Here is the successful test from 12 days ago: https://travis-ci.org/gitlabhq/gitlabhq/builds/33086108

    All that was changed in that time is that I merged in CHANGELOG.

    By Administrator on 2014-09-03T10:04:03 (imported from GitLab project)

  • Administrator
    Administrator @root ·
    Owner

    Created by: dzaporozhets

    Thanks

    By Administrator on 2014-09-03T11:03:02 (imported from GitLab project)

Please register or sign in to reply

Menu

Help