Check Role Definition for MIWI #3876

bitoku · 2024-10-01T15:49:33Z

Which issue this PR addresses:

Fixes https://issues.redhat.com/browse/ARO-10153

What this PR does / why we need it:

This PR adds a new hack script that checks the current role definitions align with the credentials requests in the release image of certain version.

The general overview of the hack script:

It takes an OCP version as an argument.
It uses the version as a standard version and assumes the credreqs are covered by the current role definitions.
It fetches the latest version of each Y version later than the given version.
Those versions are something we should validate.
It compares between the credreqs of the given version and ones of the later versions first.
This check is required because some credreqs are not supposed to be the same as the role definition because those use wildcards and the role definitions are not allowed to have them.
Thus if the new credreq is the same as the one of the given version, we can assume that the new one is also fulfilled by the current role definitions.
If there's a difference between the old and the new credreq, it fetches the corresponding roledefinition and compares them.
Once we fixed the roledefinitions, we'll update the standard version to avoid redundant checks.

After it gets merged, I'll ask someone to create a pipeline in ADO according to validate-roledef.yml, and have it run periodically (weekly or monthly).

Test plan for issue:

I already checks in the pipeline.
https://msazure.visualstudio.com/AzureRedHatOpenShift/_build/results?buildId=104604435&view=logs&j=011e1ec8-6569-5e69-4f06-baf193d1351e&t=2c2a3fcd-a26b-5bdf-f2af-a3624a4b9e28

Also you can run locally OCP_VERSION=4.14.37 make validate-roledef.

Is there any documentation that needs to be updated for this PR?

Please let me know if there is.

How do you know this will function as expected in production?

It doesn't affect production.

cadenmarchese

thank you! I have just a few questions about the automation.

.pipelines/validate-roledef.yml

hack/role/role.go

bitoku · 2024-10-15T11:19:47Z

/azp run

azure-pipelines · 2024-10-15T11:20:03Z

Azure Pipelines successfully started running 2 pipeline(s).

cadenmarchese

I tested it locally and it worked for me! I'm still concerned about the lack of checks on engineering builds or release candidate builds that aren't available in upgrade channels yet (such as 4.18.0-ec.2-x86_64) - I left a comment.

hack/role/role.go

cadenmarchese · 2024-10-15T16:59:20Z

hack/role/role.go

+
+	var vers []string
+	for ; ; v.V[1]++ {
+		res, err := http.Get(fmt.Sprintf("https://api.openshift.com/api/upgrades_info/v1/graph?channel=candidate-%s", v.MinorVersion()))


I think we need to find a way to handle engineering builds as well (for example, the 4.18 builds currently in https://quay.io/repository/openshift-release-dev/ocp-release?tab=tags). Otherwise, we end up in a tough spot with role updates. If we can't rely on the automation to tell us when a permissions change is upcoming in the upgradeable channels and instead only checks what's available for upgrade from a given version, I'm not sure how it will be used.

One potential idea - rather than looking at the upgrade graph, can the automation just compare the latest OCP build (right now, it's 4.18.0-ec.2-x86_64) with the verified-version provided to the command? We will always want to make sure our roles match the permissions extracted from the latest build.

Actually candidate channel has the rc/ec versions, but I decided to chang to use quay.io API (see the commit for details).
The only concern about rc/ec was that it's impossible to tell which is newer just based on the version.
By using quay.io, we can use last_modified date time to determine which is the latest.
Although it doesn't represent the exact release date, it's safe to assume that last modified one should be the latest version because they're pushed by CI.

This is the local output.

Details

❯ OCP_VERSION=4.14.37 make validate-roledef go run ./hack/role -verified-version "4.14.37" -oc-bin=oc Extracting 4.14.37 Checking 4.14.38 Extracting 4.14.38 Validating permissions Checking 4.15.36 Extracting 4.15.36 Validating permissions MachineApiOperator: missing actions: Microsoft.Network/virtualNetworks/join/action ImageRegistryOperator: missing actions: Microsoft.Network/privateEndpoints/write Microsoft.Network/privateEndpoints/read Microsoft.Network/privateEndpoints/privateDnsZoneGroups/write Microsoft.Network/privateEndpoints/privateDnsZoneGroups/read Microsoft.Network/privateDnsZones/read Microsoft.Network/privateDnsZones/write Microsoft.Network/privateDnsZones/join/action Microsoft.Network/privateDnsZones/A/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/read Microsoft.Network/networkInterfaces/read Microsoft.Storage/storageAccounts/PrivateEndpointConnectionsApproval/action Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action Microsoft.Network/virtualNetworks/join/action Checking 4.16.17 Extracting 4.16.17 Validating permissions MachineApiOperator: missing actions: Microsoft.Network/virtualNetworks/join/action ImageRegistryOperator: missing actions: Microsoft.Network/privateEndpoints/write Microsoft.Network/privateEndpoints/read Microsoft.Network/privateEndpoints/privateDnsZoneGroups/write Microsoft.Network/privateEndpoints/privateDnsZoneGroups/read Microsoft.Network/privateDnsZones/read Microsoft.Network/privateDnsZones/write Microsoft.Network/privateDnsZones/join/action Microsoft.Network/privateDnsZones/A/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/read Microsoft.Network/networkInterfaces/read Microsoft.Storage/storageAccounts/PrivateEndpointConnectionsApproval/action Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action Microsoft.Network/virtualNetworks/join/action ClusterIngressOperator: missing actions: Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action Checking 4.17.1 Extracting 4.17.1 Validating permissions MachineApiOperator: missing actions: Microsoft.Network/virtualNetworks/join/action ImageRegistryOperator: missing actions: Microsoft.Storage/storageAccounts/blobServices/containers/delete Microsoft.Network/privateEndpoints/write Microsoft.Network/privateEndpoints/read Microsoft.Network/privateEndpoints/privateDnsZoneGroups/write Microsoft.Network/privateEndpoints/privateDnsZoneGroups/read Microsoft.Network/privateDnsZones/read Microsoft.Network/privateDnsZones/write Microsoft.Network/privateDnsZones/join/action Microsoft.Network/privateDnsZones/A/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/read Microsoft.Network/networkInterfaces/read Microsoft.Storage/storageAccounts/PrivateEndpointConnectionsApproval/action Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action Microsoft.Network/virtualNetworks/join/action ClusterIngressOperator: missing actions: Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action AzureFilesStorageOperator: missing actions: Microsoft.Network/routeTables/join/action Checking 4.18.0-ec.2 Extracting 4.18.0-ec.2 Validating permissions MachineApiOperator: missing actions: Microsoft.Network/virtualNetworks/join/action ImageRegistryOperator: missing actions: Microsoft.Storage/storageAccounts/blobServices/containers/delete Microsoft.Network/privateEndpoints/write Microsoft.Network/privateEndpoints/read Microsoft.Network/privateEndpoints/privateDnsZoneGroups/write Microsoft.Network/privateEndpoints/privateDnsZoneGroups/read Microsoft.Network/privateDnsZones/read Microsoft.Network/privateDnsZones/write Microsoft.Network/privateDnsZones/join/action Microsoft.Network/privateDnsZones/A/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/write Microsoft.Network/privateDnsZones/virtualNetworkLinks/read Microsoft.Network/networkInterfaces/read Microsoft.Storage/storageAccounts/PrivateEndpointConnectionsApproval/action Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action Microsoft.Network/virtualNetworks/join/action ClusterIngressOperator: missing actions: Microsoft.Network/virtualNetworks/subnets/read Microsoft.Network/virtualNetworks/subnets/join/action AzureFilesStorageOperator: missing actions: Microsoft.Network/routeTables/join/action Permissions are missing in 4.15.36 Permissions are missing in 4.16.17 Permissions are missing in 4.17.1 Permissions are missing in 4.18.0-ec.2 exit status 1 make: *** [validate-roledef] Error 1

kimorris27

Nice work! I left a question about the pipeline YAML and noted a few spots where I think comments could be added or tweaked so that readers can understand things just a little bit faster.

.pipelines/validate-roledef.yml

hack/role/role.go

kimorris27 · 2024-10-18T13:29:58Z

/azp run ci

azure-pipelines · 2024-10-18T13:30:09Z

Azure Pipelines successfully started running 1 pipeline(s).

bitoku · 2024-10-18T13:31:53Z

~~ci-go failed because of #3908. CI should fail as well.~~ #3908 is merged. thanks!

bitoku · 2024-10-18T13:47:47Z

/azp run ci

azure-pipelines · 2024-10-18T13:47:58Z

Azure Pipelines successfully started running 1 pipeline(s).

bitoku · 2024-10-18T14:09:12Z

/azp run ci

azure-pipelines · 2024-10-18T14:09:23Z

Azure Pipelines successfully started running 1 pipeline(s).

dkeohane · 2024-10-23T08:13:17Z

@cadenmarchese we got this approved, can we merge?

hack/role/role.go

bitoku · 2024-10-24T13:36:14Z

/azp run ci

azure-pipelines · 2024-10-24T13:36:26Z

Azure Pipelines successfully started running 1 pipeline(s).

* bump cluster-credentials-operator * add Get to roledefinitions client * check script * pipeline * use parameters * change target-version help message * vendor * fix role.go * use candidate channel * use operator names in RP-Config * modify the output format * changed to use quay.io API * add some comments * remove pipeline resource * change role definition names

bitoku requested review from jewzaam, bennerv, hawkowl, rogbas, petrkotas, jharrington22, cblecker, cadenmarchese, UlrichSchlueter, SudoBrendan, yjst2012, jaitaiwan, anshulvermapatel, hlipsig, tiguelu, SrinivasAtmakuri, mociarain, kimorris27, tsatam and fahlmant as code owners October 1, 2024 15:49

bitoku force-pushed the miwi/role-check branch from ae4a1dc to 742b680 Compare October 1, 2024 16:05

bitoku added 4 commits October 1, 2024 17:17

bump cluster-credentials-operator

d7b4f53

add Get to roledefinitions client

f7514ee

check script

f9b5bdc

pipeline

91cc62e

bitoku force-pushed the miwi/role-check branch from 742b680 to 91cc62e Compare October 1, 2024 16:17

cadenmarchese reviewed Oct 2, 2024

View reviewed changes

.pipelines/validate-roledef.yml Outdated Show resolved Hide resolved

.pipelines/validate-roledef.yml Outdated Show resolved Hide resolved

hack/role/role.go Outdated Show resolved Hide resolved

hack/role/role.go Outdated Show resolved Hide resolved

bitoku added 2 commits October 2, 2024 16:49

use parameters

99e86e4

change target-version help message

ca6d7c1

bitoku requested a review from cadenmarchese October 3, 2024 14:24

cadenmarchese reviewed Oct 15, 2024

View reviewed changes

bitoku added 3 commits October 16, 2024 13:22

use operator names in RP-Config

57002be

modify the output format

196c942

changed to use quay.io API

87a69fe

bitoku requested a review from cadenmarchese October 16, 2024 15:14

kimorris27 reviewed Oct 16, 2024

View reviewed changes

.pipelines/validate-roledef.yml Outdated Show resolved Hide resolved

hack/role/role.go Outdated Show resolved Hide resolved

hack/role/role.go Outdated Show resolved Hide resolved

hack/role/role.go Show resolved Hide resolved

hack/role/role.go Show resolved Hide resolved

add some comments

88371bd

bitoku requested a review from kimorris27 October 18, 2024 08:21

remove pipeline resource

037d5c1

Merge remote-tracking branch 'origin/master' into miwi/role-check

50264b6

kimorris27 previously approved these changes Oct 18, 2024

View reviewed changes

cadenmarchese reviewed Oct 24, 2024

View reviewed changes

hack/role/role.go Outdated Show resolved Hide resolved

change role definition names

cd2de78

bitoku dismissed kimorris27’s stale review via cd2de78 October 24, 2024 13:35

bitoku requested a review from cadenmarchese October 24, 2024 14:19

cadenmarchese approved these changes Oct 24, 2024

View reviewed changes

cadenmarchese merged commit d89a378 into master Oct 24, 2024
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Check Role Definition for MIWI #3876

Check Role Definition for MIWI #3876

bitoku commented Oct 1, 2024

cadenmarchese left a comment

bitoku commented Oct 15, 2024

azure-pipelines bot commented Oct 15, 2024

cadenmarchese left a comment

cadenmarchese Oct 15, 2024

cadenmarchese Oct 15, 2024

bitoku Oct 16, 2024

kimorris27 left a comment

kimorris27 commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

bitoku commented Oct 18, 2024 •

edited

Loading

bitoku commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

bitoku commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

dkeohane commented Oct 23, 2024

bitoku commented Oct 24, 2024

azure-pipelines bot commented Oct 24, 2024

Check Role Definition for MIWI #3876

Check Role Definition for MIWI #3876

Conversation

bitoku commented Oct 1, 2024

Which issue this PR addresses:

What this PR does / why we need it:

Test plan for issue:

Is there any documentation that needs to be updated for this PR?

How do you know this will function as expected in production?

cadenmarchese left a comment

Choose a reason for hiding this comment

bitoku commented Oct 15, 2024

azure-pipelines bot commented Oct 15, 2024

cadenmarchese left a comment

Choose a reason for hiding this comment

cadenmarchese Oct 15, 2024

Choose a reason for hiding this comment

cadenmarchese Oct 15, 2024

Choose a reason for hiding this comment

bitoku Oct 16, 2024

Choose a reason for hiding this comment

kimorris27 left a comment

Choose a reason for hiding this comment

kimorris27 commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

bitoku commented Oct 18, 2024 • edited Loading

bitoku commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

bitoku commented Oct 18, 2024

azure-pipelines bot commented Oct 18, 2024

dkeohane commented Oct 23, 2024

bitoku commented Oct 24, 2024

azure-pipelines bot commented Oct 24, 2024

bitoku commented Oct 18, 2024 •

edited

Loading