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

add azurerm_kubernetes_cluster #54

Open
JamesWoolfenden opened this issue Aug 21, 2023 · 10 comments
Open

add azurerm_kubernetes_cluster #54

JamesWoolfenden opened this issue Aug 21, 2023 · 10 comments
Labels
enhancement New feature or request

Comments

@JamesWoolfenden
Copy link
Owner

Is your feature request related to a problem? Please describe.
A clear and concise description of what the problem is.
Ex. I'm always frustrated when [...]

Describe the solution you'd like
A clear and concise description of what you want to happen.

Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.

Additional context
Add any other context or screenshots about the feature request here.

@JamesWoolfenden JamesWoolfenden added the enhancement New feature or request label Sep 7, 2023
@Tanchwa
Copy link

Tanchwa commented Jul 9, 2024

hey I can grab this. Love the project, it's helped me stay out of hot water with the client's security guys putting in too many permissions requests...

quick question on formatting for the mapping, it looks like you just have a directory with the core resource, and inside has all the supporting resources. Is something like this correct?

.
└── mapping/
    └── azurerm/
        └── resource/
            └── kuberenetes/
                ├── azurerm_kubernetes_cluster
                ├── azurerm_kubernetes_cluster_node_pool
                └── etc

@JamesWoolfenden
Copy link
Owner Author

JamesWoolfenden commented Jul 9, 2024 via email

@JamesWoolfenden
Copy link
Owner Author

JamesWoolfenden commented Jul 10, 2024 via email

@Tanchwa
Copy link

Tanchwa commented Jul 10, 2024

I can definitely help with the Azure stuff, my department is mostly an Azure shop so it's pretty useful to have this kind of tool anyway.

Plus this forces me to make a bunch of AKS POCs that I can recommit in our internal repos

@Tanchwa
Copy link

Tanchwa commented Jul 10, 2024

One question I do have though, how are we capturing when different attributes require different perms? Can you give me an example?

Also different values for those attributes require different permissions... for example

the identity block requires either a service principal, system assigned managed identity, or user assigned identity.

For both the SP and system assigned identities, it requires Microsoft.ContainerService/managedClusters/listClusterUserCredential/action, While the User assigned requires the Microsoft.ManagedIdentity/userAssignedIdentities/assign/action

@JamesWoolfenden
Copy link
Owner Author

So the most correct answer is to try all the possible combinations to determine the permissions required for each different scenario. Which is a big job especially for a resource like the k8s cluster. So it would be best to start off trying to capture something a lot simpler first, or just add the permissions for the simplest case and then update that when you determine the others - something is better than nothing, but id rather it fail to determine enough than for the tool to over recommend ?

src/coverage/azure.md contains a link to create all the supporting files needed for each currently missing resource. resources.ps1 (i know its powershell but eh you can run ps on nix now)

run ./resource.ps1 azurerm_kubernetes_cluster in the root of the repo, feel free to add a different script if you prefer to use a different scripting language.

In terraform/azurerm there are a number of resources to help - backup contain a copy of every single resource that ive analysed for Azurerm its used as part of the testing process so if you add a new resource/datasource add an example in here. role has an example role to use against the tf in this folder.

i would start by creating the most minimal configuration possible.
If you ran the resource script you will have added a file to src/mapping/azurerm/resource called azurerm_kubernetes_cluster.json

the only anomaly in this json file is the attributes section, it has one placeholder for a common attribute tags. If the resources supports tags then it may require extra permissions to add, modify , delete this attribute.
So thats the model really if the resource supports attributes than need extra permissions you need to add a line and its permissions.

once you've managed to create the resource with the new role you will have captured the perms required to update the json file.
I hope thats clearer?

@Tanchwa
Copy link

Tanchwa commented Jul 10, 2024

no I mean all that was already clear from your readme

I mean how should the attributes look? I'm already done with the basic config, it's only 4 or 5 permissions, depending on how you slice the user identity vs system identity

[
  {
    "apply": [
      "Microsoft.ContainerService/managedClusters/read",
      "Microsoft.ContainerService/managedClusters/write",
      "Microsoft.ContainerService/managedClusters/listClusterUserCredential/action",
      "Microsoft.ManagedIdentity/userAssignedIdentities/assign/action"
    ],
    "attributes": {
      "tags": [],
      "ingressApplicationGateway": []
    },
    "destroy": [],
    "modify": [
      "Microsoft.ContainerService/managedClusters/agentPools/write"
    ],
    "plan": []
  }
]

My question for attributes is how do you want the format? Does it need to be the same as it is in terraform with snake case or match Go's/ JSON formatting?

also, if an attribute has a sub attribute, like in the case of the identity attribute, how do I add it?

identity {
    type "UserAssigned"
}

Is different from

identity {
    type = "SystemAssigned"
 }

And requires different permissions but exists under the same attribute

@JamesWoolfenden
Copy link
Owner Author

The attributes like "ingressApplicationGateway": [] are a terraform lookup so it needs to match tf.

as for a sub attribute its currently treated as a root attribute:
1 = {pike.ResourceV2}
TypeName = {string} "resource"
Name = {string} "azurerm_kubernetes_cluster"
ResourceName = {string} "example"
Provider = {string} "azurerm"
Attributes = {[]string} len:11, cap:16
0 = {string} "dns_prefix"
1 = {string} "tags"
2 = {string} "name"
3 = {string} "location"
4 = {string} "resource_group_name"
5 = {string} "default_node_pool"
6 = {string} "vm_size"
7 = {string} "name"
8 = {string} "node_count"
9 = {string} "identity"
10 = {string} "type"

So far this hasnt been an issue. So far.

@Tanchwa
Copy link

Tanchwa commented Jul 11, 2024

can I turn Attributes into a {map[string]string} so we can parse different values for a particular attribute key? Does Go support declaring multiple possible types for an item? It could be {[]string } or {map[string]string}

edit, looking in the func GetPermissionMap already gets attributes with type map[string]{}interface, so we could probably just add some logic to the GetPermissionsMap function to handle different values for the same attribute?

@JamesWoolfenden
Copy link
Owner Author

JamesWoolfenden commented Jul 12, 2024

You can try anything if it works, im not precious!
Its that or change the parsing, i havent had the issue in the other 2 clouds so some one off logic is fine, if it happens twice then its parser change although theres the overhead of making sure all other attributes are changed and parsed (not a huge deal)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants