Skip to content
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

Generate Federated Identity Credentials for MIWI Cluster #3847

Merged
merged 4 commits into from
Oct 16, 2024
Merged

Generate Federated Identity Credentials for MIWI Cluster #3847

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8b3ff35
Generate federated MIWI credentials
Sep 16, 2024
820270c
don't continue resource clean up on fed cred deletion fails
Oct 14, 2024
dc657b6
remove duplicate of clusterServicePrincipalRBAC()
Oct 14, 2024
f3b8879
nit
gouthamMN Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions hack/devtools/local_dev_env.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,8 @@ create_platform_identity_and_assign_role() {
echo "Name: $name"
echo ""

# The following operators require a role assignment since they need access to customer BYO virtual network
# RP will be responsible for other role assignments
if [[ "${operatorName}" == "MachineApiOperator" || "${operatorName}" == "NetworkOperator" \
|| "${operatorName}" == "AzureFilesStorageOperator" || "${operatorName}" == "ServiceOperator" ]]; then
# Storage Operator don't require access to customer BYO virtual network
if [[ "${operatorName}" != "StorageOperator" ]]; then
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved

assign_role_to_identity "${principalId}" "${roleDefinitionId}"
fi
Expand Down
69 changes: 69 additions & 0 deletions pkg/cluster/delete.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"strings"
"time"

"github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/msi/armmsi"
"github.com/Azure/azure-sdk-for-go/sdk/storage/azblob"
mgmtnetwork "github.com/Azure/azure-sdk-for-go/services/network/mgmt/2020-08-01/network"
mgmtfeatures "github.com/Azure/azure-sdk-for-go/services/resources/mgmt/2019-07-01/features"
Expand Down Expand Up @@ -377,6 +378,68 @@ func (m *manager) deleteClusterMsiCertificate(ctx context.Context) error {
return err
}

func (m *manager) deleteFederatedCredentials(ctx context.Context) error {
if m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile == nil {
return nil
}

if m.clusterMsiFederatedIdentityCredentials == nil {
m.log.Warning("cluster MSI federated identity credentials client is nil, trying to initialize")
err := m.initializeClusterMsiClients(ctx)
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
m.log.Errorf("cluster MSI federated identity credentials client initialization failed with error: %v", err)
return nil
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved
}
}

platformWIRolesByRoleName := m.platformWorkloadIdentityRolesByVersion.GetPlatformWorkloadIdentityRolesByRoleName()
platformWorkloadIdentities := m.doc.OpenShiftCluster.Properties.PlatformWorkloadIdentityProfile.PlatformWorkloadIdentities

for _, identity := range platformWorkloadIdentities {
_, exists := platformWIRolesByRoleName[identity.OperatorName]
if !exists {
continue
}

identityResourceId, err := azure.ParseResourceID(identity.ResourceID)
if err != nil {
return err
}

platformWIRole, exists := platformWIRolesByRoleName[identity.OperatorName]
if !exists {
continue
}

for _, sa := range platformWIRole.ServiceAccounts {
federatedIdentityCredentialResourceName, err := m.getPlatformWorkloadIdentityFederatedCredName(sa, identity)
if err != nil {
return fmt.Errorf("failed to get federated identity credential name for %s: %v", identity.ResourceID, err)
}

_, err = m.clusterMsiFederatedIdentityCredentials.Delete(
ctx,
identityResourceId.ResourceGroup,
identityResourceId.ResourceName,
federatedIdentityCredentialResourceName,
&armmsi.FederatedIdentityCredentialsClientDeleteOptions{},
)
if err != nil {
cloudErr := err.(*api.CloudError)

if cloudErr.StatusCode != http.StatusNotFound {
gouthamMN marked this conversation as resolved.
Show resolved Hide resolved
m.log.Errorf("federated identity credentials not found for %s: %v", identity.ResourceID, cloudErr.Error())
continue
} else {
return fmt.Errorf("failed to delete federated identity credentials for %s: %v", identity.ResourceID, cloudErr.Error())
}
}
}
}

return nil
}

func (m *manager) deleteResourcesAndResourceGroup(ctx context.Context) error {
resourceGroup := stringutils.LastTokenByte(m.doc.OpenShiftCluster.Properties.ClusterProfile.ResourceGroupID, '/')

Expand Down Expand Up @@ -485,6 +548,12 @@ func (m *manager) Delete(ctx context.Context) error {
}

if m.doc.OpenShiftCluster.UsesWorkloadIdentity() {
m.log.Printf("deleting platform managed identities' federated credentials")
err = m.deleteFederatedCredentials(ctx)
if err != nil {
return err
}

m.log.Printf("deleting cluster MSI certificate")
err = m.deleteClusterMsiCertificate(ctx)
if err != nil {
Expand Down
203 changes: 203 additions & 0 deletions pkg/cluster/delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ import (
mock_network "github.com/Azure/ARO-RP/pkg/util/mocks/azureclient/mgmt/network"
mock_env "github.com/Azure/ARO-RP/pkg/util/mocks/env"
mock_subnet "github.com/Azure/ARO-RP/pkg/util/mocks/subnet"
"github.com/Azure/ARO-RP/pkg/util/platformworkloadidentity"
testdatabase "github.com/Azure/ARO-RP/test/database"
utilmsi "github.com/Azure/ARO-RP/test/util/azure/msi"
"github.com/Azure/ARO-RP/test/util/deterministicuuid"
utilerror "github.com/Azure/ARO-RP/test/util/error"
)

Expand Down Expand Up @@ -486,3 +490,202 @@ func TestDeleteClusterMsiCertificate(t *testing.T) {
})
}
}

func TestDeleteFederatedCredentials(t *testing.T) {
ctx := context.Background()
docID := "00000000-0000-0000-0000-000000000000"
clusterResourceID := "/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/fakeResourceGroup/providers/Microsoft.RedHatOpenShift/openShiftClusters/fakeCluster"
mockGuid := "00000000-0000-0000-0000-000000000000"
clusterRGName := "aro-cluster"
resourceID := fmt.Sprintf("/subscriptions/%s/resourcegroups/%s/providers/Microsoft.ManagedIdentity/userAssignedIdentities/", mockGuid, clusterRGName)
fakeClint, err := utilmsi.NewTestFederatedIdentityCredentialsClient(mockGuid)

if err != nil {
fmt.Printf("failed to create fake client, err: %v\n", err)
}

tests := []struct {
name string
doc *api.OpenShiftClusterDocument
wantErr string
}{
{
name: "success - cluster doc has nil PlatformWorkloadIdentities",
doc: &api.OpenShiftClusterDocument{
ID: mockGuid,
OpenShiftCluster: &api.OpenShiftCluster{
Properties: api.OpenShiftClusterProperties{
ClusterProfile: api.ClusterProfile{
Version: "4.14.40",
},
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: ptr.To(api.UpgradeableTo("4.15.40")),
},
},
},
},
wantErr: "",
},
{
name: "success - cluster doc has non-nil but empty PlatformWorkloadIdentities",
doc: &api.OpenShiftClusterDocument{
ID: mockGuid,
OpenShiftCluster: &api.OpenShiftCluster{
Properties: api.OpenShiftClusterProperties{
ClusterProfile: api.ClusterProfile{
Version: "4.14.40",
},
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: ptr.To(api.UpgradeableTo("4.15.40")),
PlatformWorkloadIdentities: []api.PlatformWorkloadIdentity{},
},
},
},
},
wantErr: "",
},
{
name: "success - successfully delete federated credentials",
doc: &api.OpenShiftClusterDocument{
ID: docID,
OpenShiftCluster: &api.OpenShiftCluster{
ID: clusterResourceID,
Properties: api.OpenShiftClusterProperties{
ClusterProfile: api.ClusterProfile{
Version: "4.14.40",
},
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: ptr.To(api.UpgradeableTo("4.15.40")),
PlatformWorkloadIdentities: []api.PlatformWorkloadIdentity{
{
OperatorName: "CloudControllerManager",
ResourceID: fmt.Sprintf("%s/%s", resourceID, "ccm"),
},
{
OperatorName: "ClusterIngressOperator",
ResourceID: fmt.Sprintf("%s/%s", resourceID, "cio"),
},
},
},
},
},
},
wantErr: "",
},
{
name: "success - skip federated credential deletion because platform workload identities are missing the OperatorName field",
doc: &api.OpenShiftClusterDocument{
ID: docID,
OpenShiftCluster: &api.OpenShiftCluster{
ID: clusterResourceID,
Properties: api.OpenShiftClusterProperties{
ClusterProfile: api.ClusterProfile{
Version: "4.14.40",
},
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: ptr.To(api.UpgradeableTo("4.15.40")),
PlatformWorkloadIdentities: []api.PlatformWorkloadIdentity{
{
ResourceID: fmt.Sprintf("%s/%s", resourceID, "ccm"),
},
{
ResourceID: fmt.Sprintf("%s/%s", resourceID, "cio"),
},
},
},
},
},
},
wantErr: "",
},
{
name: "error - encounter blocking error deleting a federated credential",
doc: &api.OpenShiftClusterDocument{
ID: docID,
OpenShiftCluster: &api.OpenShiftCluster{
ID: clusterResourceID,
Properties: api.OpenShiftClusterProperties{
ClusterProfile: api.ClusterProfile{
Version: "4.14.40",
},
PlatformWorkloadIdentityProfile: &api.PlatformWorkloadIdentityProfile{
UpgradeableTo: ptr.To(api.UpgradeableTo("4.15.40")),
PlatformWorkloadIdentities: []api.PlatformWorkloadIdentity{
{
OperatorName: "CloudControllerManager",
ResourceID: "/subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/aro-cluster",
},
},
},
},
},
},
wantErr: "parsing failed for /subscriptions/00000000-0000-0000-0000-000000000000/resourcegroups/aro-cluster. Invalid resource Id format",
},
}

for _, tt := range tests {
uuidGen := deterministicuuid.NewTestUUIDGenerator(deterministicuuid.OPENSHIFT_VERSIONS)
dbPlatformWorkloadIdentityRoleSets, _ := testdatabase.NewFakePlatformWorkloadIdentityRoleSets(uuidGen)
f := testdatabase.NewFixture().WithPlatformWorkloadIdentityRoleSets(dbPlatformWorkloadIdentityRoleSets, uuidGen)
pir := platformworkloadidentity.NewPlatformWorkloadIdentityRolesByVersionService()
f.AddPlatformWorkloadIdentityRoleSetDocuments(&api.PlatformWorkloadIdentityRoleSetDocument{
PlatformWorkloadIdentityRoleSet: &api.PlatformWorkloadIdentityRoleSet{
Name: "testRoleSet",
Properties: api.PlatformWorkloadIdentityRoleSetProperties{
OpenShiftVersion: "4.14",
PlatformWorkloadIdentityRoles: []api.PlatformWorkloadIdentityRole{
{
OperatorName: "CloudControllerManager",
ServiceAccounts: []string{"openshift-cloud-controller-manager:cloud-controller-manager"},
},
{
OperatorName: "ClusterIngressOperator",
ServiceAccounts: []string{"openshift-ingress-operator:ingress-operator"},
},
},
},
},
},
&api.PlatformWorkloadIdentityRoleSetDocument{
PlatformWorkloadIdentityRoleSet: &api.PlatformWorkloadIdentityRoleSet{
Name: "testRoleSet",
Properties: api.PlatformWorkloadIdentityRoleSetProperties{
OpenShiftVersion: "4.15",
PlatformWorkloadIdentityRoles: []api.PlatformWorkloadIdentityRole{
{
OperatorName: "CloudControllerManager",
ServiceAccounts: []string{"openshift-cloud-controller-manager:cloud-controller-manager"},
},
{
OperatorName: "ClusterIngressOperator",
ServiceAccounts: []string{"openshift-ingress-operator:ingress-operator"},
},
},
},
},
},
)
err := f.Create()
if err != nil {
t.Fatal(err)
}

err = pir.PopulatePlatformWorkloadIdentityRolesByVersion(ctx, tt.doc.OpenShiftCluster, dbPlatformWorkloadIdentityRoleSets)
if err != nil {
t.Fatal(err)
}

t.Run(tt.name, func(t *testing.T) {
m := manager{
log: logrus.NewEntry(logrus.StandardLogger()),
doc: tt.doc,
platformWorkloadIdentityRolesByVersion: pir,
clusterMsiFederatedIdentityCredentials: fakeClint,
}

err := m.deleteFederatedCredentials(ctx)
utilerror.AssertErrorMessage(t, err, tt.wantErr)
})
}
}
Loading
Loading