Skip to content

Conversation

@ehearne-redhat
Copy link

@ehearne-redhat ehearne-redhat commented Nov 24, 2025

What

  • Ensure the canary daemon set uses its own service account, rather than the default service account.

Why

  • Using the default service account is a security issue. Using a bespoke service account ensures minimal permissions are given to the daemonset.

How

  • A service account has been created for the canary daemonset.
  • A cluster role binding has been created which references an already existing openshift-ingress-operator cluster role.
  • This cluster role binding is also attached to the service account for the canary daemon set.
  • The reconciler now detects resource creation of the cluster role binding and service account.
  • Tests have been made for the new changes.

@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 Nov 24, 2025
@openshift-ci openshift-ci bot requested review from bentito and miheer November 24, 2025 12:31
@ehearne-redhat
Copy link
Author

These tests are failing because of an improper serviceAccount mount for the ingress-canary daemonset. Working on serviceAccount and related items creation and attachment to ingress-canary daemonset.

Copy link
Author

@ehearne-redhat ehearne-redhat left a comment

Choose a reason for hiding this comment

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

I will remove the unnecessary code once tests come back.

@ehearne-redhat ehearne-redhat changed the title WIP: add ingress-operator service account to canary daemon set [WIP] OCPBUGS-65629: add ingress-operator service account to canary daemon set Nov 27, 2025
@openshift-ci-robot openshift-ci-robot added jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. labels Nov 27, 2025
@openshift-ci-robot
Copy link
Contributor

@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65629, which is invalid:

  • expected the bug to target the "4.21.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

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

Details

In response to this:

WIP - do not merge .

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/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Nov 27, 2025
@ehearne-redhat
Copy link
Author

/jira refresh

@openshift-ci-robot openshift-ci-robot added the jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. label Nov 27, 2025
@openshift-ci-robot
Copy link
Contributor

@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65629, 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.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @lihongan

Details

In response to this:

/jira refresh

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 removed the jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. label Nov 27, 2025
@openshift-ci openshift-ci bot requested a review from lihongan November 27, 2025 18:18
@ehearne-redhat ehearne-redhat changed the title [WIP] OCPBUGS-65629: add ingress-operator service account to canary daemon set [WIP] OCPBUGS-65629: ensure canary daemon set uses its own service account. Nov 27, 2025
@ehearne-redhat
Copy link
Author

/test okd-scos-images

@openshift-ci-robot
Copy link
Contributor

@ehearne-redhat: This pull request references Jira Issue OCPBUGS-65629, which is valid.

3 validation(s) were run on this bug
  • bug is open, matching expected state (open)
  • bug target version (4.21.0) matches configured target version for branch (4.21.0)
  • bug is in the state POST, 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.

Details

In response to this:

WIP - do not merge .

What

  • Ensure the canary daemon set uses its own service account, rather than the default service account.

Why

  • Using the default service account is a security issue. Using a bespoke service account ensures minimal permissions are given to the daemonset.

How

  • A service account has been created for the canary daemonset.
  • A cluster role binding has been created which references an already existing openshift-ingress-operator cluster role.
  • This cluster role binding is also attached to the service account for the canary daemon set.
  • The reconciler now detects resource creation of the cluster role binding and service account.
  • Tests have been made for the new changes.

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.

@ehearne-redhat
Copy link
Author

/test hypershift-e2e-aks

@ehearne-redhat
Copy link
Author

test e2e-aws-pre-release-ossm shows the service account is mounted and the pod is running.

ehearne-mac:must-gather (6) ehearne$ omc -n openshift-ingress-canary get pod ingress-canary-82txj -o yaml
apiVersion: v1
kind: Pod
...
  generateName: ingress-canary-
...
  name: ingress-canary-82txj
  namespace: openshift-ingress-canary
  ownerReferences:
  - apiVersion: apps/v1
    blockOwnerDeletion: true
    controller: true
    kind: DaemonSet
    name: ingress-canary
...
spec:
...
  serviceAccount: ingress-canary
  serviceAccountName: ingress-canary
...
  phase: Running
  podIP: 10.129.2.9
  podIPs:
  - ip: 10.129.2.9
  qosClass: Burstable
  startTime: "2025-11-27T18:44:24Z"

@ehearne-redhat ehearne-redhat changed the title [WIP] OCPBUGS-65629: ensure canary daemon set uses its own service account. OCPBUGS-65629: ensure canary daemon set uses its own service account. Nov 27, 2025
@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 Nov 27, 2025
@lihongan
Copy link
Contributor

/retest-required

1 similar comment
@lihongan
Copy link
Contributor

/retest-required

@ehearne-redhat
Copy link
Author

/retest

@rikatz
Copy link
Member

rikatz commented Dec 5, 2025

/lgtm cancel

@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 5, 2025
@openshift-ci-robot openshift-ci-robot removed the verified Signifies that the PR passed pre-merge verification criteria label Dec 5, 2025
@ehearne-redhat
Copy link
Author

/test e2e-aws-operator

@ehearne-redhat
Copy link
Author

/test e2e-aws-operator

@ehearne-redhat
Copy link
Author

@bentito @alebedev87 @rikatz could you take a look to see if this approach is better? :)

@bentito
Copy link
Contributor

bentito commented Dec 10, 2025

/lgtm

@bentito
Copy link
Contributor

bentito commented Dec 10, 2025

/hold for @rikatz and @Miciah to take last look too

@openshift-ci 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 Dec 10, 2025
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2025
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Dec 11, 2025

New changes are detected. LGTM label has been removed.

@ehearne-redhat
Copy link
Author

/retest

@ehearne-redhat
Copy link
Author

@bentito @rikatz @Miciah could you take a look at the addressed changes? :)

@ehearne-redhat
Copy link
Author

@bentito @rikatz @Miciah just re-pinging after winter break. If you could take a look at the addressed changes that would be great. :)

@ehearne-redhat
Copy link
Author

/test e2e-aws-pre-release-ossm

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 8, 2026

@ehearne-redhat: The following test 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-pre-release-ossm f6878ca link false /test e2e-aws-pre-release-ossm

Full PR test history. Your PR dashboard.

Details

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.

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. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. jira/severity-critical Referenced Jira bug's severity is critical for the branch this PR is targeting. jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. qe-approved Signifies that QE has signed off on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants