OCPBUGS-16181: Write manifests when AWS IAM roles already exist. #514

abutcher · 2023-02-16T15:36:24Z

No description provided.

abutcher · 2023-02-16T15:44:50Z

Holding for testing & to check if manifest generation for other clouds will have the same issue.
/hold

2uasimojo · 2023-02-16T17:02:56Z

pkg/cmd/provisioning/aws/create-iam-roles.go

@@ -227,6 +227,11 @@ func createRole(awsClient aws.Client, name string, credReq *credreqv1.Credential
 		} else {
 			role = outRole.Role
 			log.Printf("Existing role %s found", *role.Arn)


IIUC we get here if there's a role of the expected name. How confident can we be that it has the right settings? Should we validate them? What would be the right behavior if they don't match?

Delete the role and recreate it? If this is right, then we could simplify by simply doing that regardless.

Modify the role to have the right attributes? For some deltas -- like tags -- this might make sense; but for others, it could be Bad™ -- like if it would change the actual security-ness of the role.

...so probably the only right answer is to error. Which may be why the code was the way it was in the first place.

It looks like we apply policy derived from the provided CredentialsRequests on roles after this even if the role already existed so we may already be stomping the existing role policy to be what we expect based on the invocation.

codecov · 2023-02-16T18:02:23Z

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (2c3298b) 48.19% compared to head (a15b7a3) 50.46%.
Report is 59 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #514      +/-   ##
==========================================
+ Coverage   48.19%   50.46%   +2.27%     
==========================================
  Files          96       96              
  Lines       11635    12853    +1218     
==========================================
+ Hits         5607     6486     +879     
- Misses       5415     5696     +281     
- Partials      613      671      +58

Files	Coverage Δ
pkg/cmd/provisioning/aws/create-iam-roles.go	`61.13% <0.00%> (-0.59%)`	⬇️

... and 17 files with indirect coverage changes

2uasimojo · 2023-02-28T21:55:39Z

Okay, I was getting confused about what we were writing here. It's the secret in the cluster, not the policy in the cloud. This makes sense.

Add UT? Like here?

abutcher · 2023-02-28T21:59:18Z

Yep! This writes the secrets we put into the manifests directory to be read by the installer. There was some more discussion on the card so may need more changes depending on what the UX should be when the user running ccoctl aws create-all just wants to write the secrets but doesn't want to create (or update) anything in the cloud.

openshift-bot · 2023-05-30T01:00:43Z

Issues go stale after 90d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle stale

openshift-bot · 2023-06-29T08:30:57Z

Stale issues rot after 30d of inactivity.

Mark the issue as fresh by commenting /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.
Exclude this issue from closing by commenting /lifecycle frozen.

If this issue is safe to close now please do so with /close.

/lifecycle rotten
/remove-lifecycle stale

openshift-bot · 2023-07-30T00:00:41Z

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

openshift-ci · 2023-07-30T00:01:22Z

@openshift-bot: Closed this PR.

In response to this:

Rotten issues close after 30d of inactivity.

Reopen the issue by commenting /reopen.
Mark the issue as fresh by commenting /remove-lifecycle rotten.
Exclude this issue from closing again by commenting /lifecycle frozen.

/close

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-ci-robot · 2023-09-05T20:37:31Z

@abutcher: This pull request references Jira Issue OCPBUGS-16181, which is invalid:

expected the bug to target the "4.14.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.

In response to this:

CCO-335

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-ci-robot · 2023-09-05T20:37:34Z

@abutcher: This pull request references Jira Issue OCPBUGS-16181, which is invalid:

expected the bug to target the "4.14.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.

In response to this:

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.

abutcher · 2023-09-05T20:38:34Z

/jira refresh

openshift-ci-robot · 2023-09-05T20:38:36Z

@abutcher: This pull request references Jira Issue OCPBUGS-16181, which is invalid:

expected the bug to target the "4.14.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.

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 kubernetes/test-infra repository.

abutcher · 2023-09-05T20:39:29Z

/jira refresh

openshift-ci-robot · 2023-09-05T20:39:32Z

@abutcher: This pull request references Jira Issue OCPBUGS-16181, which is invalid:

expected the bug to target the "4.14.0" version, but it targets "4.14.z" instead

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.

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 kubernetes/test-infra repository.

abutcher · 2023-09-05T20:39:56Z

/jira refresh

openshift-ci-robot · 2023-09-05T20:39:59Z

@abutcher: This pull request references Jira Issue OCPBUGS-16181, which is invalid:

expected the bug to target the "4.14.0" version, but it targets "4.14.z" instead

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.

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 kubernetes/test-infra repository.

abutcher · 2023-09-12T15:35:56Z

/test e2e-aws-ovn

jstuever · 2024-01-11T18:19:38Z

/jira refresh

openshift-ci-robot · 2024-01-11T18:19:46Z

@jstuever: This pull request references Jira Issue OCPBUGS-16181, 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 ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

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.

jstuever · 2024-01-11T18:20:34Z

/assign

jstuever · 2024-01-11T18:37:29Z

/retest

jstuever · 2024-01-12T18:58:39Z

/lgtm

openshift-ci · 2024-01-12T18:59:41Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: abutcher, jstuever

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~~ [jstuever]

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

jstuever · 2024-01-12T19:00:21Z

/hold cancel

openshift-ci · 2024-01-12T20:56:29Z

@abutcher: 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-01-12T20:58:45Z

@abutcher: Jira Issue OCPBUGS-16181: All pull requests linked via external trackers have merged:

openshift/cloud-credential-operator#514

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

In response to this:

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.

jstuever · 2024-01-12T22:26:32Z

/cherry-pick release-4.15

openshift-cherrypick-robot · 2024-01-12T22:27:23Z

@jstuever: new pull request created: #653

In response to this:

/cherry-pick release-4.15

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-bot · 2024-01-14T16:20:16Z

[ART PR BUILD NOTIFIER]

This PR has been included in build ose-cloud-credential-operator-container-v4.16.0-202401141455.p0.g8bca79f.assembly.stream for distgit ose-cloud-credential-operator.
All builds following this will include this PR.

openshift-ci bot requested review from 2uasimojo and jstuever February 16, 2023 15:36

openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Feb 16, 2023

openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Feb 16, 2023

2uasimojo reviewed Feb 16, 2023

View reviewed changes

openshift-ci bot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label May 30, 2023

openshift-ci bot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jun 29, 2023

openshift-ci bot closed this Jul 30, 2023

abutcher reopened this Sep 5, 2023

abutcher changed the title ~~Write manifests when AWS IAM roles already exist.~~ OCPBUGS-16181: Write manifests when AWS IAM roles already exist. Sep 5, 2023

openshift-ci-robot added jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Sep 5, 2023

Write manifests when AWS IAM roles already exist.

870d141

Verify secret manifest created when role exists.

a15b7a3

abutcher force-pushed the CCO-335 branch from a48bd65 to a15b7a3 Compare September 7, 2023 19:22

abutcher removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Sep 12, 2023

openshift-ci-robot added jira/valid-bug Indicates that a referenced Jira bug is valid for the branch this PR is targeting. and removed jira/invalid-bug Indicates that a referenced Jira bug is invalid for the branch this PR is targeting. labels Jan 11, 2024

openshift-ci bot assigned jstuever Jan 11, 2024

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 12, 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 Jan 12, 2024

openshift-merge-bot bot merged commit 8bca79f into openshift:master Jan 12, 2024
11 checks passed

openshift-cherrypick-robot mentioned this pull request Jan 12, 2024

[release-4.15] OCPBUGS-27117: Write manifests when AWS IAM roles already exist. #653

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OCPBUGS-16181: Write manifests when AWS IAM roles already exist. #514

OCPBUGS-16181: Write manifests when AWS IAM roles already exist. #514

abutcher commented Feb 16, 2023 •

edited

abutcher commented Feb 16, 2023

2uasimojo Feb 16, 2023

abutcher Feb 16, 2023

codecov bot commented Feb 16, 2023 •

edited

2uasimojo commented Feb 28, 2023

abutcher commented Feb 28, 2023 •

edited

openshift-bot commented May 30, 2023

openshift-bot commented Jun 29, 2023

openshift-bot commented Jul 30, 2023

openshift-ci bot commented Jul 30, 2023

openshift-ci-robot commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 12, 2023

jstuever commented Jan 11, 2024

openshift-ci-robot commented Jan 11, 2024

jstuever commented Jan 11, 2024

jstuever commented Jan 11, 2024

jstuever commented Jan 12, 2024

openshift-ci bot commented Jan 12, 2024

jstuever commented Jan 12, 2024

openshift-ci bot commented Jan 12, 2024

openshift-ci-robot commented Jan 12, 2024

jstuever commented Jan 12, 2024

openshift-cherrypick-robot commented Jan 12, 2024

openshift-bot commented Jan 14, 2024

OCPBUGS-16181: Write manifests when AWS IAM roles already exist. #514

OCPBUGS-16181: Write manifests when AWS IAM roles already exist. #514

Conversation

abutcher commented Feb 16, 2023 • edited

abutcher commented Feb 16, 2023

2uasimojo Feb 16, 2023

Choose a reason for hiding this comment

abutcher Feb 16, 2023

Choose a reason for hiding this comment

codecov bot commented Feb 16, 2023 • edited

Codecov Report

2uasimojo commented Feb 28, 2023

abutcher commented Feb 28, 2023 • edited

openshift-bot commented May 30, 2023

openshift-bot commented Jun 29, 2023

openshift-bot commented Jul 30, 2023

openshift-ci bot commented Jul 30, 2023

openshift-ci-robot commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 5, 2023

openshift-ci-robot commented Sep 5, 2023

abutcher commented Sep 12, 2023

jstuever commented Jan 11, 2024

openshift-ci-robot commented Jan 11, 2024

jstuever commented Jan 11, 2024

jstuever commented Jan 11, 2024

jstuever commented Jan 12, 2024

openshift-ci bot commented Jan 12, 2024

jstuever commented Jan 12, 2024

openshift-ci bot commented Jan 12, 2024

openshift-ci-robot commented Jan 12, 2024

jstuever commented Jan 12, 2024

openshift-cherrypick-robot commented Jan 12, 2024

openshift-bot commented Jan 14, 2024

abutcher commented Feb 16, 2023 •

edited

codecov bot commented Feb 16, 2023 •

edited

abutcher commented Feb 28, 2023 •

edited