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

added valImage to monitor image vulnerabilities #1175

Closed
wants to merge 3 commits into from

Conversation

yt3liu
Copy link
Contributor

@yt3liu yt3liu commented Jul 23, 2019

What this PR does, why we need it:

  • generalized the pubsub client in monitoring
  • moved test-infra monitoring specific stuff to alert.go
  • added valImage to listen for image vulnerabilities message, send an email for all messages received

This PR does NOT create an issue in the relevant repo. This is due to when testing in my own gcp project, I didn't see anything that gives us repo specific information. Thus, for now, only send an email when receives the image vulnerability message.

Sample Email Message:

Message Data: {"name":"projects/<project-name>/occurrences/<uuid>","kind":"VULNERABILITY","notificationTime":"2019-07-23T16:54:30.902457Z"}

Pubsub Message: {
	"ID": "626966346998275",
	"Data": "<base64encoded content of the above message data>",
	"Attributes": null,
	"PublishTime": "2019-07-23T16:54:31.025Z"
}

Raw Message: &{ID:xxxxxxxxxxxxxxxxxx Data:[...] Attributes:map[] ackID:xxxxxxxxxx PublishTime:2019-07-23 16:54:31.025 +0000 UTC receiveTime:{wall:13782682632966708414 ext:29034076097 loc:0xdd5f40} size:0 calledDone:false doneFunc:0x8115e0}

Which issue(s) this PR fixes:
Part of knative/infra#136

Special notes to reviewers:

User-visible changes in this PR:

Copy link
Collaborator

@knative-prow-robot knative-prow-robot left a comment

Choose a reason for hiding this comment

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

@yt3liu: 0 warnings.

In response to this:

What this PR does, why we need it:

  • generalized the pubsub client in monitoring
  • moved test-infra monitoring specific stuff to alert.go
  • added valImage to listen for image vulnerabilities message, send an email for all messages received

This PR does NOT create an issue in the relevant repo. This is due to when testing in my own gcp project, I didn't see anything that gives us repo specific information. Thus, for now, only send an email when receives the image vulnerability message.

Sample Email Message:

Message Data: {"name":"projects/<project-name>/occurrences/<uuid>","kind":"VULNERABILITY","notificationTime":"2019-07-23T16:54:30.902457Z"}

Pubsub Message: {
  "ID": "626966346998275",
  "Data": "<base64encoded content of the above message data>",
  "Attributes": null,
  "PublishTime": "2019-07-23T16:54:31.025Z"
}

Raw Message: &{ID:xxxxxxxxxxxxxxxxxx Data:[...] Attributes:map[] ackID:xxxxxxxxxx PublishTime:2019-07-23 16:54:31.025 +0000 UTC receiveTime:{wall:13782682632966708414 ext:29034076097 loc:0xdd5f40} size:0 calledDone:false doneFunc:0x8115e0}

Which issue(s) this PR fixes:
Part of https://github.com/knative/test-infra/issues/678

Special notes to reviewers:

User-visible changes in this PR:

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@knative-prow-robot knative-prow-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 23, 2019
tools/validate-image/main.go Outdated Show resolved Hide resolved
tools/validate-image/valImage.go Outdated Show resolved Hide resolved
@chaodaiG
Copy link
Contributor

/approve

@knative-prow-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chaodaiG, yt3liu

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

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 13, 2019
# See the License for the specific language governing permissions and
# limitations under the License.

FROM golang:1.12.1
Copy link
Contributor

Choose a reason for hiding this comment

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

FROM golang:latest?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know this was an option. Will try this out :)


mailConfig, err := mail.NewMailConfig(*mailAddrSF, *mailPassSF)
if err != nil {
log.Fatal(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.Fatal(err)
log.Fatalf("Failed to create mail config: %v", err)


func validateImage(w http.ResponseWriter, r *http.Request) {
log.Printf("Serving request: %s", r.URL.Path)

Copy link
Contributor

Choose a reason for hiding this comment

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

remove blank line

go func() {
err := subClient.Receive(context.Background(), func(ctx context.Context, msg *pubsub.Message) {
log.Printf("Message: %v\n", string(msg.Data))
log.Printf("Pubsub Message: %v\n", msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

whats the point of logging both? This will log the data 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.

The one logged with string(msg.Data) prints data in a human-readable format, while printing the message directly from msg prints the message as base64encoded string.

I'd like to keep the pubsub message for debugging purposes in case there's additional attributes that gets added to the image vulnerabilities message.

# log.Printf("Message: %v\n", string(msg.Data))
Message Data: {"name":"projects/joyceyu-test/occurrences/ce3b5a33-05ed-40c2-80f2-821d38426171","kind":"DISCOVERY","notificationTime":"2019-09-12T18:04:36.630196Z"}

# log.Printf("Pubsub Message: %v\n", msg)
Pubsub Message: {
        "ID": "719306958870977",
        "Data": "eyJuYW1lIjoicHJvamVjdHMvam95Y2V5dS10ZXN0L29jY3VycmVuY2VzL2NlM2I1YTMzLTA1ZWQtNDBjMi04MGYyLTgyMWQzODQyNjE3MSIsImtpbmQiOiJESVNDT1ZFUlkiLCJub3RpZmljYXRpb25UaW1lIjoiMjAxOS0wOS0xMlQxODowNDozNi42MzAxOTZaIn0=",
        "Attributes": null,
        "PublishTime": "2019-09-12T18:04:37.058Z"
}


return &Client{
subClients: subClients,
mailClient: mconfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

we should validate mConfig != nil

log.Printf("Pubsub Message: %v\n", msg)

if time.Now().Sub(lastSent) > alertFreq {
err := c.mailClient.Send(recipients, "Image Vulnerabilities Detected", toMailContent(msg))
Copy link
Contributor

Choose a reason for hiding this comment

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

combine if
if err := ...; err != nil {}

}

func toMailContent(msg *pubsub.Message) string {
c := fmt.Sprintf("Message Data: %v\n", string(msg.Data))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we logging message and then the data from the message 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.

From the alert receiver's perspective, the pubsub message is probably not useful. I'll update the mail content to only print out the human readable message.

}

// NewValidateImageClient initialize all the resources for monitoring image vulnerabilities
func NewValidateImageClient(mconfig *mail.Config) (*Client, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Add unit test

@chaodaiG
Copy link
Contributor

chaodaiG commented Nov 4, 2019

/uncc
add me back if needed

@duglin
Copy link

duglin commented Jan 28, 2020

@yt3liu what's the status of this?

@knative-housekeeping-robot

Issues go stale after 90 days of inactivity.
Mark the issue as fresh by adding the comment /remove-lifecycle stale.
Stale issues rot after an additional 30 days of inactivity and eventually close.
If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle stale

@knative-prow-robot knative-prow-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Apr 28, 2020
@knative-housekeeping-robot

Issues go stale after 90 days of inactivity.
Mark the issue as fresh by adding the comment /remove-lifecycle stale.
Stale issues rot after an additional 30 days of inactivity and eventually close.
If this issue is safe to close now please do so by adding the comment /close.

Send feedback to Knative Productivity Slack channel or file an issue in knative/test-infra.

/lifecycle stale

@yt3liu yt3liu closed this May 5, 2020
Cynocracy pushed a commit to Cynocracy/test-infra that referenced this pull request Jun 13, 2020
* genreconciler:nonNamespaced

* Adding a force-kinds flag to the reconciler generator to allow us to generate reconcilers for non-owned types.

* force-kinds --> force-genreconciler-kinds

* some minor nits with the generators for non-knative types needed to be removed.

* remove whitespace

Co-authored-by: Nacho Cano <[email protected]>
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. lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants