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

NO-JIRA: UPSTREAM: Add several request/response headers in audit log entries #2121

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

vrutkovs
Copy link
Member

@vrutkovs vrutkovs commented Oct 22, 2024

Store several request/response headers in audit log:

  • Accept header from request in openshift.io/request-header-accept annotation
  • Accept-Encoding header from request in openshift.io/request-header-accept-encoding annotation
  • Content-Type header from response in openshift.io/response-header-content-type annotation
  • Content-Encoding header from response in openshift.io/response-header-content-encoding annotation\

These would be helpful to figure out request/response encodings (json, protobuf, raw table). This would be useful for finding inefficient clients using JSON instead of protobuf - and later on for CBOR enhancement.

openshift.io/request-header-content-length annotation would be helpful to find large responses - which may mean the client is requesting too much data etc. This annotation takes value from Content-Length header, if its unset it calculates the size of the response manually

@openshift-ci-robot openshift-ci-robot added the backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. label Oct 22, 2024
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 22, 2024
Copy link

openshift-ci bot commented Oct 22, 2024

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@openshift-ci openshift-ci bot added the vendor-update Touching vendor dir or related files label Oct 22, 2024
@vrutkovs
Copy link
Member Author

/test e2e-aws-ovn-serial

@vrutkovs
Copy link
Member Author

/payload-job-with-prs openshift/origin#29222 periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial openshift/origin#29222

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial https://github.com/openshift/origin/pull/29222,openshift/kubernetes#2121,openshift/cluster-monitoring-operator#2505

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial openshift/origin/pull/29222 #2121 openshift/cluster-monitoring-operator#2505

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: An error was encountered. No known errors were detected, please see the full error message for details.

Full error message. unable to get additional pr info from string: openshift/origin/pull/29222: string: openshift/origin/pull/29222 doesn't match expected format: org/repo#number

Please contact an administrator to resolve this issue.

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial https://github.com/openshift/origin#29222 #2121 openshift/cluster-monitoring-operator#2505

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial openshift/origin#29222 #2121 openshift/cluster-monitoring-operator#2505

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8ead4490-9117-11ef-8662-fdd14cd693cb-0

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: This PR was included in a payload test run from #2121
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/8ead4490-9117-11ef-8662-fdd14cd693cb-0

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial https://github.com/openshift/origin#29222 https://github.com/openshift/kubernetes#2121 https://github.com/openshift/cluster-monitoring-operator#2505 cluster-csi-snapshot-controller-operator#217 machine-config-operator#4658

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info.

@vrutkovs
Copy link
Member Author

/payload-job-with-prs periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial openshift/origin#29222 #2121 openshift/cluster-monitoring-operator#2505 openshift/cluster-csi-snapshot-controller-operator#217 openshift/machine-config-operator#4658

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/17dde510-911a-11ef-90f7-2ad72545de28-0

Copy link

openshift-ci bot commented Oct 23, 2024

@vrutkovs: This PR was included in a payload test run from #2121
trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command

  • periodic-ci-openshift-release-master-nightly-4.18-e2e-aws-ovn-serial

See details on https://pr-payload-tests.ci.openshift.org/runs/ci/17dde510-911a-11ef-90f7-2ad72545de28-0

@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@vrutkovs vrutkovs force-pushed the audit-log-request-response-headers branch from d829301 to be8ee97 Compare January 16, 2025 10:16
@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

…hift.io/request-header-accept-encoding" and "openshift.io/request-header-content-length"
@vrutkovs vrutkovs force-pushed the audit-log-request-response-headers branch from be8ee97 to 92ada27 Compare January 16, 2025 10:17
@vrutkovs vrutkovs changed the title Add several request/response headers in audit log entries NO-JIRA: UPSTREAM: Add several request/response headers in audit log entries Jan 16, 2025
@openshift-ci-robot
Copy link

@vrutkovs: This pull request explicitly references no jira issue.

In response to this:

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?


Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.:


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 openshift-eng/jira-lifecycle-plugin repository.

@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Jan 16, 2025
@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@openshift-ci-robot
Copy link

@vrutkovs: This pull request explicitly references no jira issue.

In response to this:

Store several request/response headers in audit log:

  • Accept header from request in openshift.io/request-header-accept annotation
  • Accept-Encoding header from request in openshift.io/request-header-accept-encoding annotation
  • Content-Type header from response in openshift.io/response-header-content-type annotation
  • Content-Encoding header from response in openshift.io/response-header-content-encoding annotation\

These would be helpful to figure out request/response encodings (json, protobuf, raw table). This would be useful for finding inefficient clients using JSON instead of protobuf - and later on for CBOR enhancement.

openshift.io/request-header-content-length annotation would be helpful to find large responses - which may mean the client is requesting too much data etc.

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 openshift-eng/jira-lifecycle-plugin repository.

@vrutkovs vrutkovs marked this pull request as ready for review January 16, 2025 10:23
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jan 16, 2025
@vrutkovs vrutkovs force-pushed the audit-log-request-response-headers branch from 92ada27 to 79ec370 Compare January 16, 2025 11:51
@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

@vrutkovs vrutkovs force-pushed the audit-log-request-response-headers branch from 79ec370 to 2e397f5 Compare January 16, 2025 12:06
@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

… audit log annotations

This adds new annotation to audit log entries for responses larger than
500 bytes
@vrutkovs vrutkovs force-pushed the audit-log-request-response-headers branch from 2e397f5 to b2bf089 Compare January 16, 2025 12:56
@openshift-ci-robot
Copy link

@vrutkovs: the contents of this pull request could not be automatically validated.

The following commits could not be validated and must be approved by a top-level approver:

Comment /validate-backports to re-evaluate validity of the upstream PRs, for example when they are merged upstream.

Copy link

openshift-ci bot commented Jan 30, 2025

@vrutkovs: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-ovn-serial b2bf089 link true /test e2e-aws-ovn-serial
ci/prow/e2e-agnostic-ovn-cmd b2bf089 link false /test e2e-agnostic-ovn-cmd
ci/prow/e2e-aws-ovn-hypershift b2bf089 link true /test e2e-aws-ovn-hypershift

Full PR test history. Your PR dashboard.

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-sigs/prow repository. I understand the commands that are listed here.

@Elbehery
Copy link

/lgtm
/approve

@Elbehery
Copy link

Elbehery commented Feb 17, 2025

/label acknowledge-critical-fixes-only

Copy link

openshift-ci bot commented Feb 17, 2025

@Elbehery: The label(s) /label acknowledge-critical-fixes-only, cannot be applied. These labels are supported: acknowledge-critical-fixes-only, platform/aws, platform/azure, platform/baremetal, platform/google, platform/libvirt, platform/openstack, ga, tide/merge-method-merge, tide/merge-method-rebase, tide/merge-method-squash, px-approved, docs-approved, qe-approved, no-qe, downstream-change-needed, rebase/manual, cluster-config-api-changed, approved, backport-risk-assessed, backports/unvalidated-commits, backports/validated-commits, bugzilla/invalid-bug, bugzilla/valid-bug, cherry-pick-approved, jira/invalid-bug, jira/valid-bug, jira/valid-reference, staff-eng-approved. Is this label configured under labels -> additional_labels or labels -> restricted_labels in plugin.yaml?

In response to this:

/label acknowledge-critical-fixes-only,

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-sigs/prow repository.

@Elbehery
Copy link

/label acknowledge-critical-fixes-only

@openshift-ci openshift-ci bot added the acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. label Feb 17, 2025
@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 17, 2025
Copy link

openshift-ci bot commented Feb 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: Elbehery, vrutkovs
Once this PR has been reviewed and has the lgtm label, please assign p0lyn0mial for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledge-critical-fixes-only Indicates if the issuer of the label is OK with the policy. backports/unvalidated-commits Indicates that not all commits come to merged upstream PRs. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. vendor-update Touching vendor dir or related files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants