Skip to content

Conversation

@osmman
Copy link
Collaborator

@osmman osmman commented Jun 16, 2025

Refs: SECURESIGN-2520

Summary by Sourcery

Extend Fulcio custom resource to support PodRequirements—including replica count, resource requirements, affinity rules, and tolerations—by updating the CRDs, API types, controller deployment logic, tests, and OLM descriptors.

New Features:

  • Allow configuring replica count via .Spec.Replicas in the Fulcio CRD
  • Allow defining resource requests and limits via .Spec.Resources
  • Allow specifying node and pod affinity rules via .Spec.Affinity
  • Allow specifying pod tolerations via .Spec.Tolerations

Enhancements:

  • Inline PodRequirements into FulcioSpec and propagate them to the deployment
  • Update Fulcio controller to apply PodRequirements when creating/updating deployments

Documentation:

  • Add OLM CRD UI descriptors for replicas, resource requirements, affinity, and tolerations in both Fulcio and SecureSign CRDs

Tests:

  • Add validation tests for .Spec.Replicas (nil, zero, positive – valid; negative – invalid)

@sourcery-ai
Copy link

sourcery-ai bot commented Jun 16, 2025

Reviewer's Guide

This PR enhances the Fulcio CRD (and the SecureSign wrapper) with routable Pod scheduling and resource customization by introducing affinity, tolerations, resources, and replicas fields, updating the API and controller to handle these PodRequirements, and adding corresponding OLM UI descriptors.

ER diagram for FulcioSpec and PodRequirements fields in CRD

erDiagram
    FULCIOSPEC ||--o| PODREQUIREMENTS : inlines
    PODREQUIREMENTS ||--o| AFFINITY : has
    PODREQUIREMENTS ||--o{ TOLERATION : has
    PODREQUIREMENTS ||--o| RESOURCEREQUIREMENTS : has
    PODREQUIREMENTS {
        int replicas
    }
    AFFINITY {
        NodeAffinity nodeAffinity
        PodAffinity podAffinity
        PodAntiAffinity podAntiAffinity
    }
    TOLERATION {
        string key
        string operator
        string value
        string effect
        int tolerationSeconds
    }
    RESOURCEREQUIREMENTS {
        map limits
        map requests
    }
    RESOURCEREQUIREMENTS ||--o{ RESOURCECLAIM : has
    RESOURCECLAIM {
        string name
        string request
    }
Loading

Class diagram for updated FulcioSpec and PodRequirements

classDiagram
    class FulcioSpec {
        +PodRequirements (inlined)
        +ExternalAccess externalAccess
        +Ctlog ctlog
        +Config config
        ...
    }
    class PodRequirements {
        +Affinity affinity
        +Toleration[] tolerations
        +ResourceRequirements resources
        +int replicas
    }
    class Affinity {
        +NodeAffinity nodeAffinity
        +PodAffinity podAffinity
        +PodAntiAffinity podAntiAffinity
    }
    class Toleration {
        +string key
        +string operator
        +string value
        +string effect
        +int tolerationSeconds
    }
    class ResourceRequirements {
        +map limits
        +map requests
        +ResourceClaim[] claims
    }
    class ResourceClaim {
        +string name
        +string request
    }
    FulcioSpec --|> PodRequirements : inlined
    PodRequirements o-- Affinity
    PodRequirements o-- Toleration
    PodRequirements o-- ResourceRequirements
    ResourceRequirements o-- ResourceClaim
Loading

File-Level Changes

Change Details Files
Extend CRD schemas to support PodRequirements
  • Add affinity (nodeAffinity, podAffinity, podAntiAffinity) block
  • Add replicas integer field with default and minimum constraints
  • Add resources object (claims, limits, requests)
  • Add tolerations array definition
config/crd/bases/rhtas.redhat.com_securesigns.yaml
config/crd/bases/rhtas.redhat.com_fulcios.yaml
Integrate PodRequirements into Fulcio API and validation
  • Inline PodRequirements struct into FulcioSpec
  • Generate DeepCopy logic for PodRequirements
  • Add unit tests for replicas validation (nil, positive, zero, negative)
api/v1alpha1/fulcio_types.go
api/v1alpha1/zz_generated.deepcopy.go
api/v1alpha1/fulcio_types_test.go
Apply PodRequirements in Fulcio controller deployment
  • Invoke PodRequirements helper when building Fulcio Deployment
internal/controller/fulcio/actions/deployment.go
Add OLM UI descriptors for new fields
  • Define alm descriptors for replicas, resources, and affinities under both Fulcio and SecureSign CRDs
config/manifests/patches/fulcio_descriptors.yaml

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@osmman osmman force-pushed the tturek/fulcio-pod-requirements branch from 748e7ff to cfab4e1 Compare June 16, 2025 11:18
Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @osmman - I've reviewed your changes - here's some feedback:

  • The OpenAPI schema for affinity, resources and tolerations is duplicated across both CRDs—consider extracting them into a shared component or referencing the upstream k8s definitions to avoid drift and reduce maintenance.
  • Tests only cover the new PodRequirements fields on the Fulcio CRD—add equivalent validation tests for replicas, resources, tolerations and affinity in the Securesign CRD as well.
  • The fulcio_descriptors.yaml patch uses hard-coded array indices for your CRD descriptors; this can break if you reorder or insert other CRDs—consider matching by name or using a strategic merge to target the right entry more robustly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The OpenAPI schema for affinity, resources and tolerations is duplicated across both CRDs—consider extracting them into a shared component or referencing the upstream k8s definitions to avoid drift and reduce maintenance.
- Tests only cover the new PodRequirements fields on the Fulcio CRD—add equivalent validation tests for replicas, resources, tolerations and affinity in the Securesign CRD as well.
- The `fulcio_descriptors.yaml` patch uses hard-coded array indices for your CRD descriptors; this can break if you reorder or insert other CRDs—consider matching by `name` or using a strategic merge to target the right entry more robustly.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Refs: SECURESIGN-2520
Signed-off-by: Tomas Turek <tturek@redhat.com>
@osmman osmman force-pushed the tturek/fulcio-pod-requirements branch from cfab4e1 to b62a13a Compare June 16, 2025 12:03
@osmman osmman requested a review from bouskaJ June 16, 2025 12:25
@osmman osmman requested review from JasonPowr and bouskaJ June 17, 2025 09:30
@osmman osmman added the enhancement New feature or request label Jun 18, 2025
@osmman osmman merged commit a46039c into main Jun 19, 2025
26 of 28 checks passed
@osmman osmman deleted the tturek/fulcio-pod-requirements branch June 19, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants