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.

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

CI/CD Deployment testing failure #648

Open
Gordonby opened this issue Aug 18, 2023 · 5 comments
Open

CI/CD Deployment testing failure #648

Gordonby opened this issue Aug 18, 2023 · 5 comments
Labels
bug Something isn't working needs-discussion An issue that needs a discussion with core maintainers to agree how to move forward

Comments

@Gordonby
Copy link
Collaborator

Gordonby commented Aug 18, 2023

Describe the bug
During full deployment tests in the CI/CD pipeline, we get an error because of the state of the environment we're deploying to.

To Reproduce

  1. Tag a PR with either;
    test-deploy-byoconfig
    test-deploy-privateconfig
  2. Wait for checks to run and fail.

Expected behavior
Environment considerations are properly reset so deployment tests can run.

Additional context

ERROR: {"status":"Failed","error":{"code":"DeploymentFailed","message":"At least one resource deployment operation failed. 
Please list deployment operations for details. Please see https://aka.ms/arm-deployment-operations for usage details.",
"details":[{"code":"BadRequest","message":"{\r\n  \"error\": {\r\n    
\"code\": \"RoleAssignmentUpdateNotPermitted\",\r\n    
\"message\": \"Tenant ID, application ID, principal ID, and scope are not allowed to be updated.\"\r\n  }\r\n}"}]}}

The role assignment thats having the problem is the RG Reader role for AppGw;

// AGIC's identity requires "Reader" permission over Application Gateway's resource group.
var reader = subscriptionResourceId('Microsoft.Authorization/roleDefinitions', 'acdd72a7-3385-48ef-bd42-f606fba81ae7')
resource appGwAGICRGReader 'Microsoft.Authorization/roleAssignments@2022-04-01' = if (ingressApplicationGateway && deployAppGw) {
  scope: resourceGroup()
  name: guid(aks.id, 'Agic', reader)
  properties: {
    roleDefinitionId: reader
    principalType: 'ServicePrincipal'
    principalId: aks.properties.addonProfiles.ingressApplicationGateway.identity.objectId
  }
}

The reason it's having a problem is because the name isn't unique. It's using a static 'Agic' string instead of an identifier to the identity such as principalId. This is because the identity is not known before main.bicep is launched, therefore it cannot form part of the name.

I see 3 options for resolution;

  1. Refactor the AppGw out of main.bicep, which will allow the role assignment name guid to be based on the already existing AKS AppGW ObjectId.
  2. Debug why the environment cleanup is not removing this role assignment (it should). Implement fix.
  3. Speak to the AKS team about addOn support for existing managed identities
@Gordonby Gordonby added the bug Something isn't working label Aug 18, 2023
@Gordonby
Copy link
Collaborator Author

Raised option 3 in the AKS repo.

Azure/AKS#3863

@samaea
Copy link
Contributor

samaea commented Aug 24, 2023

Thanks for reporting @Gordonby. + @khowling what is the impact on the above?

@Gordonby
Copy link
Collaborator Author

Gordonby commented Aug 24, 2023

Thanks for reporting @Gordonby. + @khowling what is the impact on the above?

The impact is that ;

  1. 2 of the scheduled CI jobs will always fail.
  2. Full testing in PR will always fail
  3. My Pr is blocked Bicep linter warnings for Bicep 0.20.4 #647

image

@samaea
Copy link
Contributor

samaea commented Aug 31, 2023

@mosabami to review next steps.

@mosabami mosabami added the needs-discussion An issue that needs a discussion with core maintainers to agree how to move forward label Nov 2, 2023
@mosabami
Copy link
Contributor

mosabami commented Nov 2, 2023

@pjlewisuk will be discussing this with @Gordonby and @samaea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs-discussion An issue that needs a discussion with core maintainers to agree how to move forward
Projects
None yet
Development

No branches or pull requests

3 participants