Skip to content

✨ Add RosaRoles config #5473

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

✨ Add RosaRoles config #5473

wants to merge 1 commit into from
+1,562 −65

Conversation

PanSpagetka
Copy link
Contributor

@PanSpagetka PanSpagetka commented Apr 22, 2025 •
edited
Loading

What type of PR is this?

Based on proposal #5451
Adding RosaRoleConfig API with empty Reconcile. Implementation will be added in #5499

We need to move RosaMachinePoolAutoScaling definition to controlplane, because otherwise there would be circular dependency.

/kind feature

What this PR does / why we need it:

We want to be able to create Rosa cluster with CAPA without using other tools. RosaRoleConfig CRD is adding necesary configuration for Roles and OIDC configuration, that need to be created before Rosa cluster.

Special notes for your reviewer:

Checklist:

  • squashed commits
  • includes documentation
  • includes emoji in title
  • adds unit tests
  • adds or updates e2e tests

Release note:

Add RosaRoles config API.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-priority size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Apr 22, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @PanSpagetka. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign neolit123 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

@PanSpagetka PanSpagetka marked this pull request as draft April 22, 2025 09:01
@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 2 times, most recently from 848dc7c to 59b92bf Compare April 30, 2025 11:31
serngawy
serngawy reviewed Apr 30, 2025
View reviewed changes
api/v1beta2/rosaroleconfig_types.go Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This need to go under exp directory https://github.com/kubernetes-sigs/cluster-api-provider-aws/tree/main/exp/api/v1beta2

api/v1beta2/rosaroleconfig_types.go Outdated
OIDCConfig OIDCConfig `json:"oidcConfig"`
}

// +kubebuilder:object:root=true
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// +kubebuilder:object:root=true
// +kubebuilder:object:root=true
// +kubebuilder:resource:path=rosaroleconfig,scope=cluster,categories=cluster-api,shortName=rosarole
// +kubebuilder:storageversion
// +kubebuilder:subresource:status

@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 2 times, most recently from 298567a to 46435dc Compare May 5, 2025 10:13
@PanSpagetka PanSpagetka changed the title [WIP] Add RosaRoles config ✨ Add RosaRoles config May 7, 2025
@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 2 times, most recently from a409e3c to 41feaf4 Compare May 13, 2025 08:54
serngawy
serngawy reviewed May 14, 2025
View reviewed changes
config/crd/bases/infrastructure.cluster.x-k8s.io_rosaroleconfigs.yaml Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

This is duplication I guess. one with 's' another without 's'

serngawy
serngawy reviewed May 21, 2025
View reviewed changes
@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 5 times, most recently from 013143d to f00c89f Compare May 26, 2025 11:23
@k8s-ci-robot k8s-ci-robot added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed do-not-merge/release-note-label-needed Indicates that a PR should not merge because it's missing one of the release note labels. labels May 26, 2025
@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 2 times, most recently from eada183 to 5fbe128 Compare May 28, 2025 07:47
@PanSpagetka
Copy link
Contributor Author

/test pull-cluster-api-provider-aws-verify

1 similar comment
@PanSpagetka
Copy link
Contributor Author

/test pull-cluster-api-provider-aws-verify

serngawy
serngawy reviewed May 29, 2025
View reviewed changes
Copy link
Contributor

@serngawy serngawy left a comment

Choose a reason for hiding this comment

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

one more comment but sounds good now, would you also mention in the desc that the logic is in PR-5499 so maintainers are aware of that when they review and approve.

@PanSpagetka
Copy link
Contributor Author

/test pull-cluster-api-provider-aws-verify

@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 2 times, most recently from 7029c0e to 1a32dd0 Compare June 3, 2025 12:20
@serngawy
Copy link
Contributor

serngawy commented Jun 3, 2025

@damdo would you take a look to this

@PanSpagetka
Copy link
Contributor Author

/test pull-cluster-api-provider-aws-e2e-blocking

@PanSpagetka PanSpagetka force-pushed the add-rosa-roles branch 3 times, most recently from 2e0807d to 6c33672 Compare June 16, 2025 10:14
@PanSpagetka
Copy link
Contributor Author

@damdo @nrb Could you please take a look? Thanks!

@k8s-ci-robot k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 19, 2025
@k8s-ci-robot k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jun 23, 2025
@PanSpagetka
Copy link
Contributor Author

/retest

@PanSpagetka PanSpagetka marked this pull request as draft July 15, 2025 10:27
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2025
@damdo
Copy link
Member

damdo commented Jul 17, 2025

@PanSpagetka was this replaced by a new PR?

@nrb
Copy link
Contributor

nrb commented Jul 17, 2025

@damdo #5499 instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@serngawy serngawy

@Ankitasw Ankitasw

Assignees
No one assigned
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. needs-priority ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@PanSpagetka @k8s-ci-robot @serngawy @damdo @nrb