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

Fixed azure_key_vault_secret crashes when keyvault name is camel-case Closes #637 #638

Merged
merged 2 commits into from
Jan 5, 2024

Conversation

ParthaI
Copy link
Contributor

@ParthaI ParthaI commented Jul 27, 2023

Integration test logs

Logs

No env file present for the current environment:  staging 
 Falling back to .env config
No env file present for the current environment:  staging
customEnv TURBOT_TEST_EXPECTED_TIMEOUT undefined

SETUP: tests/azure_key_vault_secret []

PRETEST: tests/azure_key_vault_secret

TEST: tests/azure_key_vault_secret
Running terraform
data.azurerm_client_config.current: Reading...
azurerm_resource_group.named_test_resource: Refreshing state... [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537]
data.azurerm_client_config.current: Read complete after 0s [id=Y2xpZW50Q29uZmlncy9jbGllbnRJZD0wNGIwNzc5NS04ZGRiLTQ2MWEtYmJlZS0wMmY5ZTFiZjdiNDY7b2JqZWN0SWQ9MDZmZDQ2YjAtYTg2Ny00OWExLWE0ZjEtZjc3Njg0NjVjYWJhO3N1YnNjcmlwdGlvbklkPWQ0NmQ3NDE2LWY5NWYtNDc3MS1iYmI1LTUyOWQ0Yzc2NjU5Yzt0ZW5hbnRJZD1jZGZmZDcwOC03ZGEwLTRjZWEtYWJlYi0wYTRjMzM0ZDdmNjQ=]
data.null_data_source.resource: Reading...
data.null_data_source.resource: Read complete after 0s [id=static]
azurerm_key_vault.named_test_resource: Refreshing state... [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537/providers/Microsoft.KeyVault/vaults/turbottest86537]
azurerm_key_vault_secret.named_test_resource: Refreshing state... [id=https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e]

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
-/+ destroy and then create replacement

Terraform will perform the following actions:

  # azurerm_key_vault.named_test_resource must be replaced
-/+ resource "azurerm_key_vault" "named_test_resource" {
      ~ access_policy                   = [
          ~ {
              - application_id          = ""
              - certificate_permissions = []
              - key_permissions         = []
              - storage_permissions     = []
                # (3 unchanged attributes hidden)
            },
        ]
      - enable_rbac_authorization       = false -> null
      - enabled_for_deployment          = false -> null
      - enabled_for_disk_encryption     = false -> null
      - enabled_for_template_deployment = false -> null
      ~ id                              = "/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537/providers/Microsoft.KeyVault/vaults/turbottest86537" -> (known after apply)
      ~ name                            = "turbottest86537" -> "turbottest76829" # forces replacement
      - purge_protection_enabled        = false -> null
      ~ resource_group_name             = "turbottest86537" -> "turbottest76829" # forces replacement
      - tags                            = {} -> null
      ~ vault_uri                       = "https://turbottest86537.vault.azure.net/" -> (known after apply)
        # (5 unchanged attributes hidden)

      - network_acls {
          - bypass                     = "AzureServices" -> null
          - default_action             = "Allow" -> null
          - ip_rules                   = [] -> null
          - virtual_network_subnet_ids = [] -> null
        }
    }

  # azurerm_key_vault_secret.named_test_resource must be replaced
-/+ resource "azurerm_key_vault_secret" "named_test_resource" {
      ~ id                      = "https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
      ~ key_vault_id            = "/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537/providers/Microsoft.KeyVault/vaults/turbottest86537" # forces replacement -> (known after apply) # forces replacement
      ~ name                    = "turbottest86537" -> "turbottest76829" # forces replacement
      ~ resource_id             = "/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537/providers/Microsoft.KeyVault/vaults/turbottest86537/secrets/turbottest86537/versions/9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
      ~ resource_versionless_id = "/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537/providers/Microsoft.KeyVault/vaults/turbottest86537/secrets/turbottest86537" -> (known after apply)
      ~ tags                    = {
          ~ "name" = "turbottest86537" -> "turbottest76829"
        }
      ~ version                 = "9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
      ~ versionless_id          = "https://turbottest86537.vault.azure.net/secrets/turbottest86537" -> (known after apply)
        # (2 unchanged attributes hidden)
    }

  # azurerm_resource_group.named_test_resource must be replaced
-/+ resource "azurerm_resource_group" "named_test_resource" {
      ~ id       = "/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537" -> (known after apply)
      ~ name     = "turbottest86537" -> "turbottest76829" # forces replacement
      - tags     = {} -> null
        # (1 unchanged attribute hidden)
    }

Plan: 3 to add, 0 to change, 3 to destroy.

Changes to Outputs:
  ~ resource_aka               = "azure://https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
  ~ resource_aka_lower         = "azure://https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
  ~ resource_id                = "https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
  ~ resource_name              = "turbottest86537" -> "turbottest76829"
  ~ secret_uri_without_version = "https://turbottest86537.vault.azure.net/secrets/turbottest86537" -> (known after apply)
  ~ secret_version             = "9389cdcdf1954198b2c19498be4f5c4e" -> (known after apply)
azurerm_key_vault_secret.named_test_resource: Destroying... [id=https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e]
azurerm_key_vault_secret.named_test_resource: Still destroying... [id=https://turbottest86537.vault.azure.net...86537/9389cdcdf1954198b2c19498be4f5c4e, 10s elapsed]
azurerm_key_vault_secret.named_test_resource: Still destroying... [id=https://turbottest86537.vault.azure.net...86537/9389cdcdf1954198b2c19498be4f5c4e, 20s elapsed]
azurerm_key_vault_secret.named_test_resource: Still destroying... [id=https://turbottest86537.vault.azure.net...86537/9389cdcdf1954198b2c19498be4f5c4e, 30s elapsed]
azurerm_key_vault_secret.named_test_resource: Destruction complete after 33s
azurerm_key_vault.named_test_resource: Destroying... [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537/providers/Microsoft.KeyVault/vaults/turbottest86537]
azurerm_key_vault.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...rosoft.KeyVault/vaults/turbottest86537, 10s elapsed]
azurerm_key_vault.named_test_resource: Destruction complete after 18s
azurerm_resource_group.named_test_resource: Destroying... [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest86537]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 10s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 20s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 30s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 40s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 50s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 1m0s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 1m10s elapsed]
azurerm_resource_group.named_test_resource: Still destroying... [id=/subscriptions/d46d7416-f95f-4771-bbb5-...c76659c/resourceGroups/turbottest86537, 1m20s elapsed]
azurerm_resource_group.named_test_resource: Destruction complete after 1m25s
azurerm_resource_group.named_test_resource: Creating...
azurerm_resource_group.named_test_resource: Creation complete after 1s [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest76829]
azurerm_key_vault.named_test_resource: Creating...
azurerm_key_vault.named_test_resource: Still creating... [10s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [20s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [30s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [40s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [50s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [1m0s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [1m10s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [1m20s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [1m30s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [1m40s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [1m50s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [2m0s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [2m10s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [2m20s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [2m30s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [2m40s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [2m50s elapsed]
azurerm_key_vault.named_test_resource: Still creating... [3m0s elapsed]
azurerm_key_vault.named_test_resource: Creation complete after 3m4s [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest76829/providers/Microsoft.KeyVault/vaults/turbottest76829]
azurerm_key_vault_secret.named_test_resource: Creating...
azurerm_key_vault_secret.named_test_resource: Creation complete after 3s [id=https://turbottest76829.vault.azure.net/secrets/turbottest76829/2a85fc95faa44164ad9ba5e12f7fe12a]

Warning: Deprecated

  with data.null_data_source.resource,
  on variables.tf line 28, in data "null_data_source" "resource":
  28: data "null_data_source" "resource" {

The null_data_source was historically used to construct intermediate values
to re-use elsewhere in configuration, the same can now be achieved using
locals

(and one more similar warning elsewhere)

Apply complete! Resources: 3 added, 0 changed, 3 destroyed.

Outputs:

location = "westus"
resource_aka = "azure://https://turbottest76829.vault.azure.net/secrets/turbottest76829/2a85fc95faa44164ad9ba5e12f7fe12a"
resource_aka_lower = "azure://https://turbottest76829.vault.azure.net/secrets/turbottest76829/2a85fc95faa44164ad9ba5e12f7fe12a"
resource_id = "https://turbottest76829.vault.azure.net/secrets/turbottest76829/2a85fc95faa44164ad9ba5e12f7fe12a"
resource_name = "turbottest76829"
secret_uri_without_version = "https://turbottest76829.vault.azure.net/secrets/turbottest76829"
secret_version = "2a85fc95faa44164ad9ba5e12f7fe12a"
subscription_id = "3510ae4d-530b-497d-8f30-53b9616fc6c1"

Running SQL query: test-get-query.sql
[
  {
    "content_type": "text",
    "enabled": true,
    "id": "https://turbottest76829.vault.azure.net/secrets/turbottest76829/2a85fc95faa44164ad9ba5e12f7fe12a",
    "name": "turbottest76829",
    "recoverable_days": 7,
    "recovery_level": "CustomizedRecoverable+Purgeable",
    "region": "westus",
    "resource_group": "turbottest76829",
    "subscription_id": "3510ae4d-530b-497d-8f30-53b9616fc6c1",
    "value": "steampipe",
    "vault_name": "turbottest76829"
  }
]
✔ PASSED

Running SQL query: test-list-query.sql
[
  {
    "id": "https://turbottest76829.vault.azure.net/secrets/turbottest76829/2a85fc95faa44164ad9ba5e12f7fe12a",
    "name": "turbottest76829"
  }
]
✔ PASSED

Running SQL query: test-not-found-query.sql
null
✔ PASSED

Running SQL query: test-turbot-query.sql
[
  {
    "akas": [
      "azure:///subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest76829/providers/Microsoft.KeyVault/vaults/turbottest76829/secrets/turbottest76829",
      "azure:///subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourcegroups/turbottest76829/providers/microsoft.keyvault/vaults/turbottest76829/secrets/turbottest76829"
    ],
    "name": "turbottest76829",
    "tags": {
      "name": "turbottest76829"
    },
    "title": "turbottest76829"
  }
]
✔ PASSED

POSTTEST: tests/azure_key_vault_secret

TEARDOWN: tests/azure_key_vault_secret

SUMMARY:

1/1 passed.

Example query results

Results
> select * from azure_key_vault_secret
+-----------------+--------------------------------------------------------------------------------------------------+-------------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+-------------->
| name            | id                                                                                               | vault_name        | enabled | managed | content_type | created_at                | expires_at | kid    | not_before | recoverable_days | recovery_leve>
+-----------------+--------------------------------------------------------------------------------------------------+-------------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+-------------->
| turbottest86537 | https://turbottest86537.vault.azure.net/secrets/turbottest86537/9389cdcdf1954198b2c19498be4f5c4e | turbottest86537   | true    | <null>  | text         | 2023-07-27T13:05:02+05:30 | <null>     | <null> | <null>     | 7                | CustomizedRec>
| CamelCase       | https://turbottest86537.vault.azure.net/secrets/CamelCase/fc5f037904104738bff8476571d20027       | turbottest86537   | true    | <null>  | <null>       | 2023-07-27T13:12:13+05:30 | <null>     | <null> | <null>     | 7                | CustomizedRec>
| test-secret     | https://keyvaultcamelcase.vault.azure.net/secrets/test-secret/7b49e5c40eff41b79f35bac546746cf5   | keyvaultcamelcase | true    | <null>  | text         | 2023-07-27T13:18:41+05:30 | <null>     | <null> | <null>     | 7                | CustomizedRec>
+-----------------+--------------------------------------------------------------------------------------------------+-------------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+-------------->

Time: 6.0s. Rows fetched: 3. Hydrate calls: 12.
> select * from azure_key_vault_secret where vault_name = 'turbottest86537' and name = 'CamelCase'
+-----------+--------------------------------------------------------------------------------------------+-----------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+---------------------------->
| name      | id                                                                                         | vault_name      | enabled | managed | content_type | created_at                | expires_at | kid    | not_before | recoverable_days | recovery_level             >
+-----------+--------------------------------------------------------------------------------------------+-----------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+---------------------------->
| CamelCase | https://turbottest86537.vault.azure.net/secrets/CamelCase/fc5f037904104738bff8476571d20027 | turbottest86537 | true    | <null>  | <null>       | 2023-07-27T13:12:13+05:30 | <null>     | <null> | <null>     | 7                | CustomizedRecoverable+Purge>
+-----------+--------------------------------------------------------------------------------------------+-----------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+---------------------------->

> select * from azure_key_vault_secret where vault_name = 'keyvaultcamelcase' and name = 'test-secret'
+-------------+------------------------------------------------------------------------------------------------+-------------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+-------------------->
| name        | id                                                                                             | vault_name        | enabled | managed | content_type | created_at                | expires_at | kid    | not_before | recoverable_days | recovery_level     >
+-------------+------------------------------------------------------------------------------------------------+-------------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+-------------------->
| test-secret | https://keyvaultcamelcase.vault.azure.net/secrets/test-secret/7b49e5c40eff41b79f35bac546746cf5 | keyvaultcamelcase | true    | <null>  | text         | 2023-07-27T13:18:41+05:30 | <null>     | <null> | <null>     | 7                | CustomizedRecoverab>
+-------------+------------------------------------------------------------------------------------------------+-------------------+---------+---------+--------------+---------------------------+------------+--------+------------+------------------+-------------------->


@@ -2,6 +2,8 @@

Azure Key Vault is a cloud service for securely storing and accessing secrets. A secret is anything that you want to tightly control access to, such as API keys, passwords, certificates, or cryptographic keys.

Note: If we intend to list the secrets by vault name using the WHERE clause (select * from azure_key_vault_secret where vault_name = 'test-vault';), the vault name must be in lowercase, even if the vault name appears in camel case in the portal.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are converting the name to lower case in the code, do we need to add this line to the doc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are converting the name to lower case in the code, do we need to add this line to the doc?

@misraved, currently, we use the ToLower function to verify the vault name both from the parent hydrate and the Secret API response. However, we are not streamlining the result to lowercase.

In the azure_key_vault table, the vault name matches the user-provided name as displayed in the portal. Nevertheless, the response from the Secret API call consistently appears in lowercase.

For those who wish to join data from both the azure_key_vault and azure_key_vault_secret tables using the vault name sourced from the azure_key_vault table, it's important to note that while data can be fetched, it might lead to empty rows. This is due to Steampipe's filtering at the level of casing, meaning that any vault name casing discrepancies will prevent the display of results.

Should any concerns or uncertainties arise, please feel free to reach out. We can sync up. Thank you!

@misraved
Copy link
Contributor

misraved commented Aug 2, 2023

@ParthaI I think it is worth reviewing this section of code -

client := keyvault.NewVaultsClientWithBaseURI(session.ResourceManagerEndpoint, subscriptionID)
client.Authorizer = session.Authorizer
maxResults := int32(100)
op, err := client.List(ctx, &maxResults)
if err != nil {
return nil, err
}
var vaultID, location string
for _, i := range op.Values() {
// The secret ID contains the Vault Name in lowercase, and here we are extracting the vault name by splitting the secret ID.
// However, if the vault name is in camel case, the current condition will not match, leading to a runtime error: index out of range [4].
// To address this issue, we should include a ToLower() check to ensure consistency and prevent potential errors.
if strings.ToLower(*i.Name) == vaultName {
vaultID = *i.ID
location = *i.Location
}
}
splitVaultID := strings.Split(vaultID, "/")
akas := []string{"azure:///subscriptions/" + subscriptionID + "/resourceGroups/" + splitVaultID[4] + "/providers/Microsoft.KeyVault/vaults/" + vaultName + "/secrets/" + splitID[4], "azure:///subscriptions/" + subscriptionID + "/resourcegroups/" + splitVaultID[4] + "/providers/microsoft.keyvault/vaults/" + vaultName + "/secrets/" + splitID[4]}
turbotData := map[string]interface{}{
"SubscriptionId": subscriptionID,
"ResourceGroup": splitVaultID[4],
"Location": location,
"Akas": akas,
}
return turbotData, nil

I don't think we need to make an additional API call to get Turbot data. We can use the ParentHydrate or the Hydrate call instead. That way we might not have an API mismatch error.

@ParthaI
Copy link
Contributor Author

ParthaI commented Aug 7, 2023

@ParthaI I think it is worth reviewing this section of code -

client := keyvault.NewVaultsClientWithBaseURI(session.ResourceManagerEndpoint, subscriptionID)
client.Authorizer = session.Authorizer
maxResults := int32(100)
op, err := client.List(ctx, &maxResults)
if err != nil {
return nil, err
}
var vaultID, location string
for _, i := range op.Values() {
// The secret ID contains the Vault Name in lowercase, and here we are extracting the vault name by splitting the secret ID.
// However, if the vault name is in camel case, the current condition will not match, leading to a runtime error: index out of range [4].
// To address this issue, we should include a ToLower() check to ensure consistency and prevent potential errors.
if strings.ToLower(*i.Name) == vaultName {
vaultID = *i.ID
location = *i.Location
}
}
splitVaultID := strings.Split(vaultID, "/")
akas := []string{"azure:///subscriptions/" + subscriptionID + "/resourceGroups/" + splitVaultID[4] + "/providers/Microsoft.KeyVault/vaults/" + vaultName + "/secrets/" + splitID[4], "azure:///subscriptions/" + subscriptionID + "/resourcegroups/" + splitVaultID[4] + "/providers/microsoft.keyvault/vaults/" + vaultName + "/secrets/" + splitID[4]}
turbotData := map[string]interface{}{
"SubscriptionId": subscriptionID,
"ResourceGroup": splitVaultID[4],
"Location": location,
"Akas": akas,
}
return turbotData, nil

I don't think we need to make an additional API call to get Turbot data. We can use the ParentHydrate or the Hydrate call instead. That way we might not have an API mismatch error.

@misraved, Thanks for pointing it out, We need an extra API call in this case.

  • Because the columns (region, resource_group, and akas) value can not be populate. because we are not getting that much information from list/get call of secret.
  • If we want to get the things using parent hydrate from the list config we may get but not from the get config.

@github-actions
Copy link

github-actions bot commented Oct 6, 2023

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale No recent activity has been detected on this issue/PR and it will be closed label Oct 6, 2023
@bigdatasourav bigdatasourav removed the stale No recent activity has been detected on this issue/PR and it will be closed label Nov 6, 2023
Copy link
Contributor

@bigdatasourav bigdatasourav left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bigdatasourav bigdatasourav merged commit fc2a0f5 into main Jan 5, 2024
1 check passed
@bigdatasourav bigdatasourav deleted the issue-637 branch January 5, 2024 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants