OCPBUGS-27023: add infrastructure annotations #128

alebedev87 · 2024-01-17T09:14:51Z

According to the Red Hat Operator's Best practices the infrastructure annotations are now required. The old "infrastructure-features" annotation is deprecated hence I removed it.

openshift-ci-robot · 2024-01-17T09:14:57Z

@alebedev87: This pull request references Jira Issue OCPBUGS-27023, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

bug is open, matching expected state (open)
bug target version (4.16.0) matches configured target version for branch (4.16.0)
bug is in the state New, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

According to the Red Hat Operator's Best practices the infrastructure annotations are now required. The old "infrastructure-features" annotation is deprecated hence I removed it.

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.

alebedev87 · 2024-01-17T12:28:31Z

/retest

Miciah · 2024-01-17T16:22:38Z

config/manifests/bases/aws-load-balancer-operator.clusterserviceversion.yaml

+    features.operators.openshift.io/disconnected: "true"
+    features.operators.openshift.io/fips-compliant: "true"
+    features.operators.openshift.io/proxy-aware: "true"
+    features.operators.openshift.io/tls-profiles: "false"
+    features.operators.openshift.io/token-auth-aws: "true"
+    features.operators.openshift.io/token-auth-azure: "false"
+    features.operators.openshift.io/token-auth-gcp: "false"
    olm.skipRange: <1.1.0
    operatorframework.io/suggested-namespace: aws-load-balancer-operator
-    operators.openshift.io/infrastructure-features: '["proxy-aware"]'


Can you describe in the commit message why you are adding the additional capabilities, and cite the relevant commits or Jira tickets related to the newly added capabilities?

The commit message changed.

candita · 2024-01-17T16:59:09Z

/assign
/assign @Miciah

lihongan · 2024-01-18T07:48:41Z

bundle/manifests/aws-load-balancer-operator.clusterserviceversion.yaml

@@ -66,9 +66,15 @@ metadata:
        }
      ]
    capabilities: Basic Install
+    features.operators.openshift.io/disconnected: "true"
+    features.operators.openshift.io/fips-compliant: "true"


The fips bug OCPBUGS-14846 has been fixed by PR #99 but that's for latest v1.1.0.
If this PR is back ported to old release then we need to update this annotation to "false" I think.

Got catch! Yes, I'll need to backport this PR down to 1.0. So the 1.0 PR needs to have fips-compliant set to false.

I just want to double-check that this is correct.

We set CGO_ENABLED=0 in the Dockerfile:

aws-load-balancer-operator/Dockerfile

Line 17 in 1a2c320

RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build -a -o manager main.go

We don't set CGO_ENABLED in the Makefile:

aws-load-balancer-operator/Makefile

Line 178 in 1a2c320

go build -mod=vendor -o bin/manager main.go

The Dockerfile is referenced here: https://github.com/openshift/release/blob/8703107e5b40d8559b2d7858350a88e3634762a6/ci-operator/config/openshift/aws-load-balancer-operator/openshift-aws-load-balancer-operator-main.yaml#L17

Do we use the Dockerfile or the Makefile? Is CGO_ENABLED set or not, and if it is, to what value?

My understanding based on openshift/external-dns-operator#213 (comment), is that we need CGO_ENABLED=1 (or maybe not setting CGO_ENABLED at all is sufficient?) in order to use OpenSSL, and using OpenSSL (from UBI) is sufficient to claim FIPS compliance using the features.operators.openshift.io/fips-compliant: "true" annotation. Do I understand correctly?

(By the way, aws-load-balancer-controller doesn't specify CGO_ENABLED at all in its Dockerfile or Makefile.)

Do we use the Dockerfile or the Makefile?

You found the right CI config, we use Dockerfile to build the operator image which is then added to the OLM bundle which in turn is deployed for each e2e test.

Is CGO_ENABLED set or not, and if it is, to what value?

It's set to 0 which means that we use a statically built operator for the e2e tests. Also the users of the GitHub version of ALBO may use Dockerfile and build not FIPS compliant operator binary. Then what we deliver to the customers is built on CPaaS where we don't set CGO_ENABLED so the operator is built for the dynamic linkage.

My understanding based on openshift/external-dns-operator#213 (comment), is that we need CGO_ENABLED=1 (or maybe not setting CGO_ENABLED at all is sufficient?) in order to use OpenSSL, and using OpenSSL (from UBI) is sufficient to claim FIPS compliance using the features.operators.openshift.io/fips-compliant: "true" annotation. Do I understand correctly?

Default value for CGO_ENABLED is 1, so it's enough to not set it. Whether the UBI image is fine the FIPS compliance or not is not 100% clear to me. I tend to say yes as it's RHEL based. Asked a question in FIPS workshop slides.

Let me remove CGO_ENABLED=0 from Dockerfile.

CGO_ENABLED=0 removed from Dockerfile.

#133 fixed the FIPS compliance for ALBO. I think we have the right to set true now.

alebedev87 · 2024-01-19T13:41:32Z

/retest

alebedev87 · 2024-02-01T15:53:28Z

/test e2e-aws-proxy-operator

Miciah · 2024-02-15T01:54:55Z

Hm, maybe removing CGO_ENABLED=0 was not the right thing to do. CI is now failing, with the aws-load-balancer-operator-controller-manager pod logging the following:

/manager: /lib64/libc.so.6: version `GLIBC_2.32' not found (required by /manager)
/manager: /lib64/libc.so.6: version `GLIBC_2.34' not found (required by /manager)

By the way, is there any reason not to set terminationMessagePolicy: FallbackToLogsOnError on all pod containers? David Eads has been making that change to a lot of other operators (see OCPBUGS-28230) in order to make this sort of failure a little easier to spot, so I just now posted #131 to do the same for this operator.

Miciah · 2024-02-15T05:39:05Z

Oh, do we need to resolve OCPBUGS-24653 aws-load-balancer-operator fails FIPS check-payload scan before we can specify features.operators.openshift.io/fips-compliant: "true"?

candita · 2024-02-15T19:11:33Z

bundle/manifests/aws-load-balancer-operator.clusterserviceversion.yaml

+    features.operators.openshift.io/fips-compliant: "true"
+    features.operators.openshift.io/proxy-aware: "true"
+    features.operators.openshift.io/tls-profiles: "false"
+    features.operators.openshift.io/token-auth-aws: "true"


Only proxy-aware is set as a true infrastructure features in the previous version. Can you explain why you mark token-auth-aws and fips-compliant as true here?

The commit message now explains why these features are marked as true.

What a phrase - "human obliviousness" !

alebedev87 · 2024-04-02T07:50:06Z

/test e2e-aws-proxy-operator

alebedev87 · 2024-04-02T14:18:11Z

e2e-aws-rosa-operator is failing due to an ongoing issue with the OCP 4.16 candidate integration into ROSA.

The downstream (Red Hat) OLM requires a new set of annotations to describe the features supported by Red Hat operators. This commit introduces the necessary changes to implement these annotations, aiming to enhance the user experience when customers use the OperatorHub in the OpenShift cluster. The previous convention used the "infrastructure-features" annotation as a mere list, lacking a way to differentiate between deliberate absence of a value and human obliviousness. This limitation led to its deprecation. With the new set of annotations, we address this issue. During the transition, the old list was mapped to the new set of annotations, with some of the new features set to "false" as they are not yet implemented. The "token-auth-aws" feature is set to "true", as it is addressed in the context of the STS standardization effort(https://issues.redhat.com/browse/NE-1307). The "fips-compliant" feature is set to "true" following verification during the resolution of https://issues.redhat.com/browse/OCPBUGS-14846 and https://issues.redhat.com//browse/OCPBUGS-24653.

alebedev87 · 2024-04-02T15:29:58Z

A link to OCPBUGS-24653 has been added to the commit message.

Miciah · 2024-04-02T17:39:26Z

Thanks!
/approve
/lgtm
/hold
for @candita.

openshift-ci · 2024-04-02T17:39:58Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Miciah

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:

~~OWNERS~~ [Miciah]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

alebedev87 · 2024-04-04T13:55:36Z

/test e2e-aws-rosa-operator

candita · 2024-04-05T14:43:23Z

LGTM
/unhold

openshift-ci-robot · 2024-04-05T16:56:23Z

/retest-required

Remaining retests: 0 against base HEAD 5d9d2fa and 2 for PR HEAD 167b573 in total

openshift-ci · 2024-04-05T18:58:58Z

@alebedev87: 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-ci-robot · 2024-04-05T19:01:30Z

@alebedev87: Jira Issue OCPBUGS-27023: All pull requests linked via external trackers have merged:

openshift/aws-load-balancer-operator#128

Jira Issue OCPBUGS-27023 has been moved to the MODIFIED state.

In response to this:

According to the Red Hat Operator's Best practices the infrastructure annotations are now required. The old "infrastructure-features" annotation is deprecated hence I removed it.

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 added jira/severity-low Referenced Jira bug's severity is low for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Jan 17, 2024

openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Jan 17, 2024

openshift-ci bot requested review from lihongan, candita and Miciah January 17, 2024 09:15

alebedev87 force-pushed the feature-annotations branch from d1894d4 to 5562c83 Compare January 17, 2024 16:16

Miciah reviewed Jan 17, 2024

View reviewed changes

openshift-ci bot assigned candita and Miciah Jan 17, 2024

lihongan reviewed Jan 18, 2024

View reviewed changes

alebedev87 force-pushed the feature-annotations branch from 5562c83 to 76a9c5e Compare January 19, 2024 10:21

alebedev87 changed the title ~~OCPBUGS-27023: add infrastructure annotations~~ [WIP] OCPBUGS-27023: add infrastructure annotations Jan 19, 2024

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 Jan 19, 2024

alebedev87 changed the title ~~[WIP] OCPBUGS-27023: add infrastructure annotations~~ OCPBUGS-27023: add infrastructure annotations Jan 19, 2024

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 19, 2024

alebedev87 force-pushed the feature-annotations branch 2 times, most recently from 7c71eac to c1eb9e4 Compare January 29, 2024 14:48

candita reviewed Feb 15, 2024

View reviewed changes

alebedev87 force-pushed the feature-annotations branch from 7430a8e to b61fb56 Compare March 29, 2024 19:59

alebedev87 force-pushed the feature-annotations branch from b61fb56 to 167b573 Compare April 2, 2024 15:29

openshift-ci bot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged. labels Apr 2, 2024

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2024

openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 5, 2024

openshift-merge-bot bot merged commit 0baccef into openshift:main Apr 5, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-27023: add infrastructure annotations #128

OCPBUGS-27023: add infrastructure annotations #128

alebedev87 commented Jan 17, 2024

openshift-ci-robot commented Jan 17, 2024

alebedev87 commented Jan 17, 2024

Miciah Jan 17, 2024

alebedev87 Jan 29, 2024

candita commented Jan 17, 2024

lihongan Jan 18, 2024

alebedev87 Jan 18, 2024

Miciah Feb 13, 2024

Miciah Feb 13, 2024

alebedev87 Feb 13, 2024

alebedev87 Feb 13, 2024

alebedev87 Apr 2, 2024

alebedev87 commented Jan 19, 2024

alebedev87 commented Feb 1, 2024

Miciah commented Feb 15, 2024 •

edited

Miciah commented Feb 15, 2024

candita Feb 15, 2024

Miciah Mar 22, 2024

candita Apr 5, 2024

alebedev87 commented Apr 2, 2024

alebedev87 commented Apr 2, 2024

alebedev87 commented Apr 2, 2024 •

edited

Miciah commented Apr 2, 2024

openshift-ci bot commented Apr 2, 2024

alebedev87 commented Apr 4, 2024

candita commented Apr 5, 2024

openshift-ci-robot commented Apr 5, 2024

openshift-ci bot commented Apr 5, 2024

openshift-ci-robot commented Apr 5, 2024

OCPBUGS-27023: add infrastructure annotations #128

OCPBUGS-27023: add infrastructure annotations #128

Conversation

alebedev87 commented Jan 17, 2024

openshift-ci-robot commented Jan 17, 2024

alebedev87 commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

candita commented Jan 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 commented Jan 19, 2024

alebedev87 commented Feb 1, 2024

Miciah commented Feb 15, 2024 • edited

Miciah commented Feb 15, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alebedev87 commented Apr 2, 2024

alebedev87 commented Apr 2, 2024

alebedev87 commented Apr 2, 2024 • edited

Miciah commented Apr 2, 2024

openshift-ci bot commented Apr 2, 2024

alebedev87 commented Apr 4, 2024

candita commented Apr 5, 2024

openshift-ci-robot commented Apr 5, 2024

openshift-ci bot commented Apr 5, 2024

openshift-ci-robot commented Apr 5, 2024

Miciah commented Feb 15, 2024 •

edited

alebedev87 commented Apr 2, 2024 •

edited