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

Allow release event #62

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 16 additions & 16 deletions src/main/java/org/jenkinsci/plugins/gogs/GogsWebHook.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,19 +23,7 @@ associated documentation files (the "Software"), to deal in the Software without

package org.jenkinsci.plugins.gogs;

import hudson.Extension;
import hudson.model.Job;
import hudson.model.UnprotectedRootAction;
import hudson.security.ACL;
import hudson.util.Secret;
import net.sf.json.JSONObject;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.apache.commons.codec.binary.Hex;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;
import static com.google.common.base.Preconditions.checkNotNull;

import javax.crypto.Mac;
import javax.crypto.spec.SecretKeySpec;
Expand All @@ -50,7 +38,19 @@ associated documentation files (the "Software"), to deal in the Software without
import java.util.Map;
import java.util.logging.Logger;

import static com.google.common.base.Preconditions.checkNotNull;
import hudson.Extension;
import hudson.model.Job;
import hudson.model.UnprotectedRootAction;
import hudson.security.ACL;
import hudson.util.Secret;
import net.sf.json.JSONObject;
import org.acegisecurity.context.SecurityContext;
import org.acegisecurity.context.SecurityContextHolder;
import org.apache.commons.codec.binary.Hex;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.StaplerResponse;

/**
* @author Alexander Verhaar
Expand Down Expand Up @@ -106,8 +106,8 @@ public void doIndex(StaplerRequest req, StaplerResponse rsp) throws IOException

// Get X-Gogs-Event
String event = req.getHeader("X-Gogs-Event");
if (!"push".equals(event)) {
result.setStatus(403, "Only push event can be accepted.");
if (!"push".equals(event) && !"release".equals(event)) {
result.setStatus(403, "Only push or release events are accepted.");
Comment on lines +109 to +110

Choose a reason for hiding this comment

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

Can the create event be added as well ?
e.g.

if (!"push".equals(event) && !"release".equals(event) && !"create".equals(event)) {
    result.setStatus(403, "Only push, release or create events are accepted.");

Copy link

@huangluyu huangluyu Feb 18, 2020

Choose a reason for hiding this comment

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

Can the pull_request event be added as well ?
e.g.

if (!"push".equals(event) && !"release".equals(event) && !"create".equals(event) && !"pull_request".equals(event)) {
    result.setStatus(403, "Only push, release, create or pull_request events are accepted.");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same here as the above comment. Adding other events breaks things at the moment. Need to investigate how to fix this.

Copy link

Choose a reason for hiding this comment

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

@sanderv32 very interested in this work being completed and merged. Is there something I can contribute to help?

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 whole thing broke down with this changes. Haven't looked for a long time at this change, but if you want to test please do so.

exitWebHook(result, rsp);
return;
}
Expand Down