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 webhooks for all harbor projects when created #1753

Merged
merged 3 commits into from
Apr 6, 2020
Merged

Conversation

cdchris12
Copy link
Contributor

Checklist

  • Affected Issues have been mentioned in the Closing issues section
  • Documentation has been written/updated
  • PR title is ready for changelog and subsystem label(s) applied

This commit adds the ability for kubernetesbuilddeploy to automatically add a webhook for each project when that project is created. These webhooks will be triggered when a container security scan within Harbor either fails or completes.

@cdchris12 cdchris12 added 9-security Security subsystem 2-build-deploy Build & Deploy subsystem labels Mar 31, 2020
@cdchris12 cdchris12 self-assigned this Mar 31, 2020
Comment on lines 126 to 130
if (lagoonHarborRoute === 'http://172.17.0.1:8084'){
var webhookAddress = 'http://172.17.0.1:7777'
} else {
var webhookAddress = "https://hooks.lagoon.amazeeio.cloud/"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be done a bit better:

  1. instead of using http://172.17.0.1:7777 we can use http://webhook-handler:3000 as harbor and the webhook-handler are locally in the same docker network you can talk to the services with their DNS names and their internal ports.
  2. The discovery of the public Hook WebHook URL can be fully automated via checking the env variables, see an example:
    https://github.com/amazeeio/lagoon/blob/b6117c46d9fbc335b0dd0126a633904a70548486/services/api/src/clients/keycloakClient.js#L21-L26

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with you on how to set the default for this variable. About the container address; we already use the 172.17.0.1 notation on line 7, which why I opted for it here. I'll test with webhook-handler to verify it works as expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The webhook-handler:3000 works as expected. The only problem with that is that we aren't loading the webhook-handler for most of the CI tests. Should I write some testing logic which includes these webhooks as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After sleeping on it, I realized that we can't really test this just yet because we do not yet have a webhook handler for these Harbor webhooks as of yet.

@Schnitzel Schnitzel added this to the v1.5.0 milestone Mar 31, 2020
@Schnitzel Schnitzel merged commit fba83ed into master Apr 6, 2020
@Schnitzel Schnitzel modified the milestones: v1.5.0, v1.4.1 Apr 6, 2020
@Schnitzel Schnitzel deleted the harbor_webhooks branch April 6, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2-build-deploy Build & Deploy subsystem 9-security Security subsystem
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants