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

deps: Bump go autorest #44

Merged
merged 3 commits into from Oct 10, 2020

Conversation

sgreene570
Copy link

@sgreene570 sgreene570 commented Oct 7, 2020

Azure/go-autorest recently switched from using the unmaintained dgrijalva/jwt-go to a newly maintained fork, form3tech-oss/jwt-go.
Azure/go-autorest is the only coredns dependency that required dgrijalva/jwt-go, which has recently been identified as vulnerable via CVE-2020-26160. While Azure/go-autorest did not make use of any of the vulnerable code in dgrijalva/jwt-go, this bump essentially removes dgrijalva/jwt-go from the vendor/ directory to prevent security auditing tools from raising concern over the not-needed vulnerable code.


This PR also fixes the coredns binary regex in the .gitignore. The prior regex (^coredns$) was not working for me on my system for some reason.

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 7, 2020
@sgreene570
Copy link
Author

/retest

.gitignore Outdated
@@ -1,7 +1,7 @@
query.log
Corefile
*.swp
^coredns$
coredns
Copy link

Choose a reason for hiding this comment

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

Won't this match vendor/github.com/coredns?

Copy link
Author

Choose a reason for hiding this comment

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

Ah, yea, Ill drop this change

Copy link

Choose a reason for hiding this comment

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

Maybe ./coredns would be what you want?

Copy link
Author

Choose a reason for hiding this comment

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

I think /coredns will work. Ill add the gitignore commit back.

@@ -1,5 +1,4 @@
package chaos

// Owners are all GitHub handlers of all maintainers.
var Owners = []string{"Miciah", "bradbeam", "chrisohaver", "danehans", "dilyevsky", "ekleiner", "fastest963", "greenpau", "grobie", "inigohu", "ironcladlou", "isolus", "johnbelamaric", "knobunc", "miekg", "nchrisdk", "nitisht", "pmoroney", "rajansandeep", "rdrozhdzh", "rtreffer", "smarterclayton", "superq", "varyoo", "yongtang"}

var Owners = []string{"bradbeam", "chrisohaver", "darshanime", "dilyevsky", "ekleiner", "fastest963", "greenpau", "grobie", "ihac", "inigohu", "isolus", "johnbelamaric", "miekg", "nchrisdk", "nitisht", "pmoroney", "rajansandeep", "rdrozhdzh", "rtreffer", "stp-ip", "superq", "varyoo", "ykhr53", "yongtang"}
Copy link

Choose a reason for hiding this comment

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

Is this change intentional?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, let me fix that.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed. This is not the first time i've accidentally commited changes to zowners.go. Seems like I somehow accidentally ran make gen.

Azure/go-autorest recently switched from using the unmaintained
dgrijalva/jwt-go to a newly maintained fork, form3tech-oss/jwt-go.
Azure/go-autorest is the only coredns dependency that required
dgrijalva/jwt-go, which has recently been identified as vulnerable via
CVE-2020-26160. While Azure/go-autorest did not make use of any of
the vulnerable code in dgrijalva/jwt-go, this bump essentially removes
dgrijalva/jwt-go from the vendor/ directory to prevent security auditing
tools from raising concern over the not-needed vulnerable code.
@sgreene570
Copy link
Author

@Miciah updated this PR per your comments. Thanks!

@sgreene570
Copy link
Author

/retest

@Miciah
Copy link

Miciah commented Oct 10, 2020

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 10, 2020
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah, sgreene570

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

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

1 similar comment
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-merge-robot openshift-merge-robot merged commit 738dda7 into openshift:master Oct 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants