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

Ignore private security forks in label_sync #15199

Merged

Conversation

@cblecker
Copy link
Member

cblecker commented Nov 8, 2019

We can't modify labels on private security forks, so filter them out.

/assign @BenTheElder

fixes #15157

@cblecker cblecker force-pushed the cblecker:private-security-label-sync-fix branch from 9fa0aca to 8c51b12 Nov 8, 2019
@@ -319,6 +319,10 @@ func GetOrg(org string) (string, bool) {
return org, false
}

// securityForkNameRE is a regexp matching repos that are temporary security forks
// https://help.github.com/en/github/managing-security-vulnerabilities/collaborating-in-a-temporary-private-fork-to-resolve-a-security-vulnerability
var securityForkNameRE = regexp.MustCompile(`^[\w-]+-ghsa-[\w-]+$`)

This comment has been minimized.

Copy link
@BenTheElder

BenTheElder Nov 8, 2019

Member

it's a little unfortunate that we have to ignore some magical pattern :/

we should probably document this, however I'd prefer to get label_sync working and have someone follow up

This comment has been minimized.

Copy link
@cblecker

cblecker Nov 8, 2019

Author Member

I couldn't see any extra flag in the v3 API that calls out that this is a private security fork.

This comment has been minimized.

Copy link
@BenTheElder

BenTheElder Nov 8, 2019

Member

same. IMO this is a reasonable fix to an unreasonable problem to have. It's not great that any repo with this string will magically be ignored but 🤷‍♂

This comment has been minimized.

Copy link
@BenTheElder

BenTheElder Nov 8, 2019

Member

feedback for github really

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Nov 8, 2019

@k8s-ci-robot k8s-ci-robot added the lgtm label Nov 8, 2019
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

k8s-ci-robot commented Nov 8, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: BenTheElder, cblecker

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit 7bf2a7a into kubernetes:master Nov 8, 2019
5 checks passed
5 checks passed
cla/linuxfoundation cblecker authorized
Details
pull-test-infra-bazel Job succeeded.
Details
pull-test-infra-verify-file-perms Job succeeded.
Details
pull-test-infra-yamllint Job succeeded.
Details
tide In merge pool.
Details
@k8s-ci-robot k8s-ci-robot added this to the v1.17 milestone Nov 8, 2019
@cblecker cblecker deleted the cblecker:private-security-label-sync-fix branch Nov 8, 2019
@cjwagner

This comment has been minimized.

Copy link
Member

cjwagner commented Nov 8, 2019

Did we reach out to GH support about this yet?

@BenTheElder

This comment has been minimized.

Copy link
Member

BenTheElder commented Nov 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.