Skip to content

Conversation

@maqiuyujoyce
Copy link
Collaborator

@maqiuyujoyce maqiuyujoyce commented Mar 25, 2025

Change description

ReCAPTCHAEnterpriseFirewallPolicy is supported based on the GRPC client.

Tests you have done

  • [N/A] Run make ready-pr to ensure this PR is ready for review.
  • Perform necessary E2E testing for changed resources.

These is no commit for test result against mock API because there is no diff to commit.

log.V(2).Info("updating FirewallPolicy", "name", a.id)
mapCtx := &direct.MapContext{}

desiredPb := ReCAPTCHAEnterpriseFirewallPolicySpec_ToProto(mapCtx, &a.desired.Spec)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you double check if user can specify name in the Create call. If so, this logic need to be updated to support custom id

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The resource has service-generated ID so the name shouldn't be set during Create. Even if it is set, the value won't be respected by the API.

Copy link
Collaborator

@yuwenma yuwenma left a comment

Choose a reason for hiding this comment

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

/approve

One question to confirm, otherwise looks good

@google-oss-prow
Copy link
Contributor

[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

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@maqiuyujoyce maqiuyujoyce force-pushed the resource-recaptchaenterprise-firewallpolicy branch from 5699bd0 to 841af85 Compare April 3, 2025 22:22
@barney-s
Copy link
Collaborator

barney-s commented Apr 5, 2025

/lgtm

@google-oss-prow google-oss-prow bot added the lgtm label Apr 5, 2025
@google-oss-prow google-oss-prow bot merged commit e5c9d3f into GoogleCloudPlatform:master Apr 5, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants