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

Getting an error as column 'id' requires hydrate data from getKeyVaultSecret but none is available on azure_key_vault_secret table. Closes #104 #111

Merged
merged 7 commits into from
May 4, 2021

Conversation

anisadas
Copy link
Contributor

@anisadas anisadas commented Apr 28, 2021

Integration test logs

Logs
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: Refreshing state...
data.null_data_source.resource: Refreshing state...
azurerm_resource_group.named_test_resource: Creating...
azurerm_resource_group.named_test_resource: Creation complete after 4s [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest12044]
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: Creation complete after 2m33s [id=/subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourceGroups/turbottest12044/providers/Microsoft.KeyVault/vaults/turbottest12044]
azurerm_key_vault_secret.named_test_resource: Creating...
azurerm_key_vault_secret.named_test_resource: Creation complete after 5s [id=https://turbottest12044.vault.azure.net/secrets/turbottest12044/4aa5a2cbec2e4cc3a3122e70b34d1bec]

Warning: Deprecated 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


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

Outputs:

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

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

Running SQL query: test-list-query.sql
[
  {
    "id": "https://turbottest12044.vault.azure.net/secrets/turbottest12044/4aa5a2cbec2e4cc3a3122e70b34d1bec",
    "name": "turbottest12044"
  }
]
✔ 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/turbottest12044/providers/Microsoft.KeyVault/vaults/turbottest12044/secrets/turbottest12044",
      "azure:///subscriptions/3510ae4d-530b-497d-8f30-53b9616fc6c1/resourcegroups/turbottest12044/providers/microsoft.keyvault/vaults/turbottest12044/secrets/turbottest12044"
    ],
    "name": "turbottest12044",
    "tags": {
      "name": "turbottest12044"
    },
    "title": "turbottest12044"
  }
]
✔ PASSED

POSTTEST: tests/azure_key_vault_secret

TEARDOWN: tests/azure_key_vault_secret

SUMMARY:

1/1 passed.

Example query results

Results
Add example SQL query results here (please include the input queries as well)

…tSecret but none is available on azure_key_vault_secret table. Closes #104
@anisadas anisadas marked this pull request as ready for review April 28, 2021 14:35
@anisadas anisadas requested review from Subhajit97 and LalitLab and removed request for Subhajit97 April 28, 2021 14:35
Comment on lines +206 to +208
if !*data.Attributes.Enabled {
return nil, nil
}
Copy link
Contributor

@Subhajit97 Subhajit97 Apr 29, 2021

Choose a reason for hiding this comment

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

What will happen if someone has passed quals to make GET call, i.e. select * from azure_key_vault_secret where name = 'secret123' and vault_name = 'vault123'?

Copy link
Contributor

@Subhajit97 Subhajit97 left a comment

Choose a reason for hiding this comment

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

@anisadas I have left a comment. Can you please verify the GET function and try to query the table with the quals required for GET call?

@anisadas
Copy link
Contributor Author

@Subhajit97 I already mentioned in this issue that GET operation is not allowed on disable secret if someone tries to query select * from azure_key_vault_secret where name = 'secrettest123' and vault_name = 'vaulttest1234'; on disable secret it throws this error

Error: keyvault.BaseClient#GetSecret: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="Operation get is not allowed on a disabled secret." InnerError={"code":"SecretDisabled"}

This query works only on enable secret.

@Subhajit97
Copy link
Contributor

@Subhajit97 I already mentioned in this issue that GET operation is not allowed on disable secret if someone tries to query select * from azure_key_vault_secret where name = 'secrettest123' and vault_name = 'vaulttest1234'; on disable secret it throws this error

Error: keyvault.BaseClient#GetSecret: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="Operation get is not allowed on a disabled secret." InnerError={"code":"SecretDisabled"}

This query works only on enable secret.

Yes, but if someone runs a query without knowing the resource state, it should not return any error.

Error: keyvault.BaseClient#GetSecret: Failure responding to request: StatusCode=403 -- Original Error: autorest/azure: Service returned an error. Status=403 Code="Forbidden" Message="Operation get is not allowed on a disabled secret." InnerError={"code":"SecretDisabled"}

Instead of returning error, we should safely handle this error.

Copy link
Contributor

@Subhajit97 Subhajit97 left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -21,7 +21,7 @@ func tableAzureKeyVaultSecret(_ context.Context) *plugin.Table {
Get: &plugin.GetConfig{
KeyColumns: plugin.AllColumns([]string{"vault_name", "name"}),
Hydrate: getKeyVaultSecret,
ShouldIgnoreError: isNotFoundError([]string{"ResourceNotFound", "404"}),
ShouldIgnoreError: isNotFoundError([]string{"ResourceNotFound", "404", "403"}),
Copy link
Contributor

Choose a reason for hiding this comment

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

What else can give us 403 errors from the getKeyVaultSecret function? If we have incorrect permissions, do we also get a 403 error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes when wrong access policy set on vault in that time also get 403 error

Copy link
Contributor

Choose a reason for hiding this comment

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

@anisadas Can you please try looking for the Operation get is not allowed on a disabled secret. error message instead of the 403 error code, as we don't want to ignore all permission errors for the get operation.

@cbruno10 cbruno10 merged commit e809b3c into main May 4, 2021
@cbruno10 cbruno10 deleted the issue104 branch May 4, 2021 18:49
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