search

GoogleCloudPlatform / k8s-config-connector

GCP Config Connector, a Kubernetes add-on for managing GCP resources
https://cloud.google.com/config-connector/docs/overview
Apache License 2.0
901 stars 235 forks source link

Ignore NotFound error when deleting PrivilegedAccessManagerEntitlement #3255

Closed maqiuyujoyce closed 4 days ago

maqiuyujoyce commented 5 days ago

Change description

The deletion of PrivilegedAccessManagerEntitlement resource is stuck if the underlying GCP resource doesn't exist. But this deletion should go through successfully. Ignoring the resource not found error in the deletion fixes this error.

Tests you have done

maqiuyujoyce commented 5 days ago

A test step that deletes the underlying GCP resource first and then delete the KRM resource will help us detect such error.

yuwenma commented 5 days ago

What is the real PAM service behavior if the DELETE request is on a non-exist resource?

... is stuck if the underlying GCP resource doesn't exist. Is this from the dynamic test? Do you know why the resource does not exist?

yuwenma commented 5 days ago

/hold

maqiuyujoyce commented 5 days ago

What is the real PAM service behavior if the DELETE request is on a non-exist resource?

... is stuck if the underlying GCP resource doesn't exist. Is this from the dynamic test? Do you know why the resource does not exist?

It'll return a NOT_FOUND error:

$ gcloud beta pam entitlements delete pam-3zpy4jkelr4buo --location global
Parsed [entitlement] resource: projects/[myproject]/locations/global/entitlements/pam-3zpy4jkelr4buo
All grants associated with the entitlement are also deleted upon deleting the entitlement.

Do you want to continue (Y/n)?  Y

ERROR: (gcloud.beta.pam.entitlements.delete) NOT_FOUND: Resource 'projects/[myproject]/locations/global/entitlements/pam-3zpy4jkelr4buo' was not found. This command is authenticated as [user] which is the active account specified by the [core/account] property
yuwenma commented 4 days ago

What is the real PAM service behavior if the DELETE request is on a non-exist resource?

... is stuck if the underlying GCP resource doesn't exist. Is this from the dynamic test? Do you know why the resource does not exist?

It'll return a NOT_FOUND error:

$ gcloud beta pam entitlements delete pam-3zpy4jkelr4buo --location global
Parsed [entitlement] resource: projects/[myproject]/locations/global/entitlements/pam-3zpy4jkelr4buo
All grants associated with the entitlement are also deleted upon deleting the entitlement.

Do you want to continue (Y/n)?  Y

ERROR: (gcloud.beta.pam.entitlements.delete) NOT_FOUND: Resource 'projects/[myproject]/locations/global/entitlements/pam-3zpy4jkelr4buo' was not found. This command is authenticated as [user] which is the active account specified by the [core/account] property

Could you elaborate more on this change? If the GCP server returns a not found error, the MockGCP should not silo that .

maqiuyujoyce commented 4 days ago

@yuwenma this change is not about mockGCP (sorry for the confusing communication offline), but is about the direct controller.

Deletion of a KRM object should be successful when the underlying GCP resource is gone. If the underlying GCP resource never exists, then the deletion should go through successfully. So we should ignore the NOT_FOUND error returned by the GCP API.

yuwenma commented 4 days ago

/lgtm /approve /hold cancel

google-oss-prow[bot] commented 4 days ago

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yuwenma

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files: - ~~[OWNERS](https://github.com/GoogleCloudPlatform/k8s-config-connector/blob/master/OWNERS)~~ [yuwenma] Approvers can indicate their approval by writing `/approve` in a comment Approvers can cancel approval by writing `/approve cancel` in a comment