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.

Sign up for GitHub

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

Jump to bottom

Fix auditor #187

Merged
merged 2 commits into from
Mar 4, 2020
Merged

Fix auditor #187

merged 2 commits into from
Mar 4, 2020

Conversation

listx
Copy link
Contributor

@listx listx commented Mar 4, 2020

These address #184 and #185.

Linus Arver added 2 commits March 4, 2020 12:31
auditor: add missing argument to %v formatting log
f01bf9a 
We could log something more specific, but for now just log the entire
gcrPayload.
auditor: always generate UUID
a0c71b8 
If the UUID hasn't been provided on in the environment, generate one
automatically.
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Mar 4, 2020
@k8s-ci-robot k8s-ci-robot requested review from dims and tpepper March 4, 2020 20:46
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: listx

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 added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 4, 2020
@listx
Copy link
Contributor Author

listx commented Mar 4, 2020

After this merges, I have to (1) promote the cip-auditor image to production (probably the cip image as well), and then (2) redeploy the auditor on Cloud Run.

@listx
Copy link
Contributor Author

listx commented Mar 4, 2020

/cc @thockin

@k8s-ci-robot k8s-ci-robot requested a review from thockin March 4, 2020 21:04
tpepper
tpepper reviewed Mar 4, 2020
View reviewed changes
lib/audit/auditor.go
@@ -392,7 +392,7 @@ func (s *ServerContext) Audit(w http.ResponseWriter, r *http.Request) {
// If we can't find the source registry for this image, then reject the
// transaction.
if string(srcRegistry.Name) == "" {
msg := fmt.Sprintf("(%s) TRANSACTION REJECTED: could not determine source registry: %v", s.ID)
msg := fmt.Sprintf("(%s) TRANSACTION REJECTED: could not determine source registry: %v", s.ID, gcrPayload)
Copy link
Member

Choose a reason for hiding this comment

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

Is there any chance of over-logging with gcrPayload? Sharing something that shouldn't be in the log?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the GCR payload only has very sparse details --- the image path, digest, and action (e.g., INSERT, DELETE)

@tpepper
Copy link
Member

tpepper commented Mar 4, 2020

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 4, 2020
@k8s-ci-robot k8s-ci-robot merged commit 2070ba0 into kubernetes-sigs:master Mar 4, 2020
listx pushed a commit to listx/k8s.io that referenced this pull request Mar 4, 2020
promote promoter
c28f4c2 
This brings in the fixes from
kubernetes-sigs/promo-tools#187.
@listx listx mentioned this pull request Mar 4, 2020
Merged
listx pushed a commit to listx/k8s.io that referenced this pull request Mar 5, 2020
promote promoter
d6d301b 
Included fixes:

kubernetes-sigs/promo-tools#187
kubernetes-sigs/promo-tools#189
This was referenced Mar 5, 2020
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tpepper tpepper tpepper left review comments

@dims dims Awaiting requested review from dims

@thockin thockin Awaiting requested review from thockin

Assignees

@tpepper tpepper

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/release Categorizes an issue or PR as relevant to SIG Release. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@listx @k8s-ci-robot @tpepper