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

LOG-4604: Add infrastructure annotations #2285

Merged
merged 1 commit into from
Jan 5, 2024

Conversation

jcantrill
Copy link
Contributor

Description

This PR:

  • adds infrastructure annotations

@cahartma is it correct that we can claim aws token support? We can use the tokens as provided by the CloudCredentialsOperator?

@syedriko is it correct we can claim FIPS compliance based upon the following definition?

Whether the opperator accepts the [FIPS-140](https://issues.redhat.com/browse/FIPS-140) configuration of the underlying platform and works on nodes that are booted into FIPS mode. In this mode, the operator and any workloads it manages (operands) are solely calling the RHEL cryptographic library submitted for [FIPS-140](https://issues.redhat.com/browse/FIPS-140) validation.

Links

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

openshift-ci-robot commented Dec 8, 2023

@jcantrill: This pull request references LOG-4604 which is a valid jira issue.

Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the task to target either version "4.8." or "openshift-4.8.", but it targets "Logging 5.8.z" instead.

In response to this:

Description

This PR:

  • adds infrastructure annotations

@cahartma is it correct that we can claim aws token support? We can use the tokens as provided by the CloudCredentialsOperator?

@syedriko is it correct we can claim FIPS compliance based upon the following definition?

Whether the opperator accepts the [FIPS-140](https://issues.redhat.com/browse/FIPS-140) configuration of the underlying platform and works on nodes that are booted into FIPS mode. In this mode, the operator and any workloads it manages (operands) are solely calling the RHEL cryptographic library submitted for [FIPS-140](https://issues.redhat.com/browse/FIPS-140) validation.

Links

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.

@jcantrill
Copy link
Contributor Author

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 8, 2023
@jcantrill
Copy link
Contributor Author

/cherrypick release-5.8
/cherrypick release-5.7
/cherrypick release-5.6

@openshift-cherrypick-robot

@jcantrill: once the present PR merges, I will cherry-pick it on top of release-5.8 in a new PR and assign it to you.

In response to this:

/cherrypick release-5.8
/cherrypick release-5.7
/cherrypick release-5.6

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.

Copy link
Contributor

openshift-ci bot commented Dec 8, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jcantrill

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-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 8, 2023
features.operators.openshift.io/cni: "false"
features.operators.openshift.io/csi: "false"
features.operators.openshift.io/disconnected: "true"
features.operators.openshift.io/fips-compliant: "true"
Copy link

@anpingli anpingli Dec 12, 2023

Choose a reason for hiding this comment

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

I don't think we can claim fips-compliant=true without fips scan.

According the latest scan result. we have to fix the issue below

logging-view-plugin-container 5.5 to 5.7 | /opt/app-root/plugin-backend | go binary is not CGO_ENABLED 
logging-view-plugin-container 5.8  | /opt/app-root/plugin-backend | go binary has no build tags set (should have strictfipsruntime)  
log-file-metric-exporter-container | /usr/local/bin/log-file-metric-exporter | go binary has no build tags set (should have strictfipsruntime) 
cluster-logging-operator-container | /usr/bin/cluster-logging-operator | go binary has no build tags set (should have strictfipsruntime) | 

AFAIK, there are tool to scan golang and java based image. No idea how to scan fluentd? No idea how to scan vector?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@anpingli I would like to emphasize our criteria for true/false is based upon the provided definition of the annotation:

Whether the opperator accepts the [FIPS-140](https://issues.redhat.com/browse/FIPS-140) configuration of the underlying platform and works on nodes that are booted into FIPS mode. In this mode, the operator and any workloads it manages (operands) are solely calling the RHEL cryptographic library submitted for [FIPS-140](https://issues.redhat.com/browse/FIPS-140) validation

If scanning is the only way to confirm ⬆️ then we need the scan, but I do not believe this is what it says

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is this text accurately represents "our" definition, and needs to be set to true in order for our operator to show in the filtered catalog list.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also think we need to set this to true and work on the scan in the background if we need to.

Copy link

Choose a reason for hiding this comment

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

I think we should use fips-compliant=false before the scan issue to be fixed. https://issues.redhat.com/browse/LOG-4886

Copy link

Choose a reason for hiding this comment

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

The QE/ART/Security team is planning to scan the operator according to fips-compliant. it may block the release if scan failed when fips-compliant=true

@jcantrill
Copy link
Contributor Author

continue to hold pending larger conversation of what we can properly claim

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 4, 2024
@openshift-merge-robot openshift-merge-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 4, 2024
@jcantrill
Copy link
Contributor Author

/hold cancel

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 4, 2024
@cahartma
Copy link
Contributor

cahartma commented Jan 5, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 5, 2024
Copy link
Contributor

openshift-ci bot commented Jan 5, 2024

@jcantrill: all tests passed!

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

@openshift-merge-bot openshift-merge-bot bot merged commit 328a7ab into openshift:master Jan 5, 2024
10 checks passed
@jcantrill jcantrill deleted the log4604_2 branch January 5, 2024 16:10
@openshift-cherrypick-robot

@jcantrill: #2285 failed to apply on top of branch "release-5.8":

Applying: LOG-4604: Add infrastructure annotations
Using index info to reconstruct a base tree...
M	bundle/manifests/clusterlogging.clusterserviceversion.yaml
M	config/manifests/bases/clusterlogging.clusterserviceversion.yaml
Falling back to patching base and 3-way merge...
Auto-merging config/manifests/bases/clusterlogging.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in config/manifests/bases/clusterlogging.clusterserviceversion.yaml
Auto-merging bundle/manifests/clusterlogging.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in bundle/manifests/clusterlogging.clusterserviceversion.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 LOG-4604: Add infrastructure annotations
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-5.8
/cherrypick release-5.7
/cherrypick release-5.6

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.

@openshift-cherrypick-robot

@jcantrill: #2285 failed to apply on top of branch "release-5.7":

Applying: LOG-4604: Add infrastructure annotations
Using index info to reconstruct a base tree...
M	bundle/manifests/clusterlogging.clusterserviceversion.yaml
M	config/manifests/bases/clusterlogging.clusterserviceversion.yaml
Falling back to patching base and 3-way merge...
Auto-merging config/manifests/bases/clusterlogging.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in config/manifests/bases/clusterlogging.clusterserviceversion.yaml
Auto-merging bundle/manifests/clusterlogging.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in bundle/manifests/clusterlogging.clusterserviceversion.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 LOG-4604: Add infrastructure annotations
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-5.8
/cherrypick release-5.7
/cherrypick release-5.6

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.

@openshift-cherrypick-robot

@jcantrill: #2285 failed to apply on top of branch "release-5.6":

Applying: LOG-4604: Add infrastructure annotations
Using index info to reconstruct a base tree...
M	bundle/manifests/clusterlogging.clusterserviceversion.yaml
M	config/manifests/bases/clusterlogging.clusterserviceversion.yaml
Falling back to patching base and 3-way merge...
Auto-merging config/manifests/bases/clusterlogging.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in config/manifests/bases/clusterlogging.clusterserviceversion.yaml
Auto-merging bundle/manifests/clusterlogging.clusterserviceversion.yaml
CONFLICT (content): Merge conflict in bundle/manifests/clusterlogging.clusterserviceversion.yaml
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 LOG-4604: Add infrastructure annotations
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

In response to this:

/cherrypick release-5.8
/cherrypick release-5.7
/cherrypick release-5.6

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.

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. 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. release/5.9
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants