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

Fix the last of the lint errors and add lint checking to presubmits. #867

Merged
merged 1 commit into from
May 16, 2019

Conversation

dlorenc
Copy link
Contributor

@dlorenc dlorenc commented May 15, 2019

Changes

This commit adds the golangci-lint and errcheck.txt exclude files, and
runs golangci-lint during presubmits. It also fixes the last of the lint
errors!

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

See the contribution guide
for more details.

@googlebot googlebot added the cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit label May 15, 2019
@tekton-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dlorenc

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

@tekton-robot tekton-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 15, 2019
@abayer
Copy link
Contributor

abayer commented May 15, 2019

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 15, 2019
@tekton-robot tekton-robot removed the lgtm Indicates that a PR is ready to be merged. label May 15, 2019
@dlorenc
Copy link
Contributor Author

dlorenc commented May 15, 2019

Fixes: #794

Blocked by: tektoncd/plumbing#19

@dlorenc
Copy link
Contributor Author

dlorenc commented May 15, 2019

/test pull-tekton-pipeline-build-tests

@dlorenc
Copy link
Contributor Author

dlorenc commented May 15, 2019

Just to make sure this actually works I'm going to purposefully add a lint error, see the failure, then fix it.

This commit adds the golangci-lint and errcheck.txt exclude files, and
runs golangci-lint during presubmits. It also fixes the last of the lint
errors!
@dlorenc
Copy link
Contributor Author

dlorenc commented May 15, 2019

Yay, it failed right!

@dlorenc
Copy link
Contributor Author

dlorenc commented May 15, 2019

/test pull-tekton-pipeline-integration-tests

@dibyom
Copy link
Member

dibyom commented May 16, 2019

/lgtm

@tekton-robot tekton-robot added the lgtm Indicates that a PR is ready to be merged. label May 16, 2019
@tekton-robot tekton-robot merged commit c65e64e into tektoncd:master May 16, 2019
pradeepitm12 pushed a commit to openshift/tektoncd-pipeline that referenced this pull request Jan 27, 2021
Storing the body as `json.RawMessage` can lead to loss of information about
spaces in the incoming body when the InterceptorRequest is marshaled as JSON
before sending it over HTTP.

This is because Go's `json.Marshal` will encode `[]byte` as a base64 string and
base64 does not preserve spaces. Adding a custom `MarshalJSON` does not help
here since `MarshalJSON` will return a `[]byte` that will then get compacted as
base64 by `json.Marshal`.

This loss of spaces can be problematic for some use cases. For instance, the
GitHub payload signature validation requires us to compare using the exact body
as it was sent. Otherwise, the validation fails. Note that this will only be a
problem when we marshal the InterceptorRequest i.e. when we move the core
interceptors out to its own server.

Using a string means that the string will be quoted e.g. `{\"foo\": \"bar\"}`.
Go should take care of unquoting it during the unmarshaling process. However,
intercetpor authors using a different language will have to manually unquote
the string by parsing it as a JSON object.

Part of #271, tektoncd#867

Signed-off-by: Dibyo Mukherjee <[email protected]>
pradeepitm12 pushed a commit to openshift/tektoncd-pipeline that referenced this pull request Jan 27, 2021
The EventListener will forward events over HTTP to the interceptor.  Each core
interceptor is available at a different path e.g. /cel for CEL.

Fixes tektoncd#867

Signed-off-by: Dibyo Mukherjee <[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. cla: yes Trying to make the CLA bot happy with ppl from different companies work on one commit lgtm Indicates that a PR is ready to be merged. 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.

5 participants