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

Bug: ASO should not try to delete undeletable resources #3975

Open
theunrepentantgeek opened this issue Apr 29, 2024 · 1 comment
Open

Bug: ASO should not try to delete undeletable resources #3975

theunrepentantgeek opened this issue Apr 29, 2024 · 1 comment
Assignees
Labels
high-priority Issues we intend to prioritize (security, outage, blocking bug)
Milestone

Comments

@theunrepentantgeek
Copy link
Member

Version of Azure Service Operator

v2.7.0 (and earlier)

Describe the bug

Discovered while testing API version 2023-11-15 of documentdb - the type SqlDatabaseContainerThroughputSetting may not be deleted directly via ARM.

ASO knows this already, as we generate the correct list of supported operations in sql_database_container_throughput_setting_types_gen.go:

// GetSupportedOperations returns the operations supported by the resource
func (setting *SqlDatabaseContainerThroughputSetting) GetSupportedOperations() []genruntime.ResourceOperation {
	return []genruntime.ResourceOperation{
		genruntime.ResourceOperationGet,
		genruntime.ResourceOperationPut,
	}
}

However, we don't respect that when running, resulting in noisy errors:

test_logger.go:160: I2024-04-28T22:19:01Z] SqlDatabaseThroughputSettingController 
    "msg"="Error during Delete" 
    "err"="deleting resource "/subscriptions/82acd5bb-4206-47d4-9c12-a65db028483d/resourceGroups/asotest-rg-aokpof/providers/Microsoft.DocumentDB/databaseAccounts/sample-sqldb-account/sqlDatabases/sample-sql-db/throughputSettings/default": DELETE https://management.azure.com/subscriptions/82acd5bb-4206-47d4-9c12-a65db028483d/resourceGroups/asotest-rg-aokpof/providers/Microsoft.DocumentDB/databaseAccounts/sample-sqldb-account/sqlDatabases/sample-sql-db/throughputSettings/default
    --------------------------------------------------------------------------------
    RESPONSE 405: 405 Method Not Allowed
    ERROR CODE: MethodNotAllowed
    --------------------------------------------------------------------------------
    {
      "code": "MethodNotAllowed",
      "message": "Message: The requested verb is not supported."
    }
    --------------------------------------------------------------------------------
    " name="sample-sql-throughput" 
    namespace="aso-test-samples-creationanddeletion-test-sqldatabase-v1a-7e1fc" 
    azureName="default" 
    action="BeginDelete"

Expected behavior

We should respect the available verbs for the resource, and skip deletion from Azure where it's not possible, even though this may produce results surprising for some users.

@theunrepentantgeek theunrepentantgeek added the high-priority Issues we intend to prioritize (security, outage, blocking bug) label Apr 29, 2024
@theunrepentantgeek theunrepentantgeek added this to the v2.8.0 milestone Apr 29, 2024
@theunrepentantgeek
Copy link
Member Author

Decision: Block direct deletion, returning an error for the condition indicating that direct deletion can't be done, must delete the parent. This avoids noisy error messages in the logs.

Two ways to approach - one is to block async, which makes delete a one-way operation. The other is to modify the webhook, which may have issues with cascading deletes from the parent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high-priority Issues we intend to prioritize (security, outage, blocking bug)
Projects
Development

No branches or pull requests

1 participant