NE-2723: Migrate to aws-sdk-go-v2#1445
Conversation
|
@rikatz: This pull request references NE-2723 which is a valid jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (292)
📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (2)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThe PR migrates the codebase from AWS SDK for Go v1 to AWS SDK for Go v2. It updates module dependencies, rewrites the AWS Route53 DNS provider to use v2 clients and APIs (including tagging-based hosted zone lookup and partition-aware endpoint resolution), updates tests and e2e utilities to v2 client/types, replaces deprecated ioutil usages with os/io equivalents, and adjusts shared-credentials handling to v2 config loading. New helpers for partition/region detection and endpoint resolution were added and provider instances now cache resolved service endpoints. 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/e2e/lb_eip_test.go (1)
443-505:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn false when EIP release fails or unassociated EIPs still exist
Line 495 can return success even when releases failed (Line 489) or when unassociated EIPs were only attempted to be released. That can stop polling early and leak EIPs.
💡 Suggested fix
func cleanupEIPAllocations(t *testing.T, svc *ec2.Client, clusterName string) bool { @@ - // Release each unassociated EIP + // Release each unassociated EIP + releaseFailed := false for _, eip := range unassociatedEIPs { _, err := svc.ReleaseAddress(context.TODO(), &ec2.ReleaseAddressInput{ AllocationId: eip.AllocationId, }) if err != nil { t.Errorf("Failed to release EIP %v with Allocation ID %v: %v", aws.ToString(eip.PublicIp), aws.ToString(eip.AllocationId), err) + releaseFailed = true } else { t.Logf("Released EIP %v with Allocation ID %v", aws.ToString(eip.PublicIp), aws.ToString(eip.AllocationId)) } } - if len(associatedEIPs) == 0 { + // Keep polling until previously-unassociated EIPs are gone from a fresh DescribeAddresses call. + if releaseFailed || len(unassociatedEIPs) > 0 { + return false + } + if len(associatedEIPs) == 0 { t.Log("No associated EIPs found with the specified tag key and value.") return true } else {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/e2e/lb_eip_test.go` around lines 443 - 505, cleanupEIPAllocations currently can return true even when releasing unassociated EIPs failed or when unassociated EIPs were only attempted to be released; update the function to track release failures and only return true when there were no release errors and there are no remaining unassociated EIPs (and associatedEIPs is empty). Concretely: inside cleanupEIPAllocations, add a boolean (e.g., releaseFailed) set to true whenever svc.ReleaseAddress returns an error (instead of just t.Errorf), and after the release loop treat any releaseFailed or any non-empty unassociatedEIPs as a failure path (return false); only return true when associatedEIPs is empty AND releaseFailed is false. Reference: cleanupEIPAllocations, unassociatedEIPs, associatedEIPs, svc.ReleaseAddress.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@test/e2e/lb_eip_test.go`:
- Around line 443-505: cleanupEIPAllocations currently can return true even when
releasing unassociated EIPs failed or when unassociated EIPs were only attempted
to be released; update the function to track release failures and only return
true when there were no release errors and there are no remaining unassociated
EIPs (and associatedEIPs is empty). Concretely: inside cleanupEIPAllocations,
add a boolean (e.g., releaseFailed) set to true whenever svc.ReleaseAddress
returns an error (instead of just t.Errorf), and after the release loop treat
any releaseFailed or any non-empty unassociatedEIPs as a failure path (return
false); only return true when associatedEIPs is empty AND releaseFailed is
false. Reference: cleanupEIPAllocations, unassociatedEIPs, associatedEIPs,
svc.ReleaseAddress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 2c5b648a-c0c5-417c-ba11-1ec530d375b3
⛔ Files ignored due to path filters (292)
go.sumis excluded by!**/*.sumvendor/github.com/aws/aws-sdk-go-v2/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/NOTICE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/accountid_endpoint_mode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/arn/arn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/checksum.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credential_cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/auto.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/configuration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/defaults.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaults/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/defaultsmode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/endpoints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/from_ptr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/logging.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/logging_generate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/osname_go115.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/recursion_detection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/request_id_retriever.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/middleware/user_agent.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/ec2query/error_utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/array.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/encoder.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/map.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/object.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/query/value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/restjson/decoder_util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/protocol/xml/error_utils.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/none.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_bucket.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/ratelimit/token_rate_limit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/request.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_ratelimit.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/adaptive_token_bucket.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/attempt_metrics.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/jitter_backoff.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retry.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/retryable_error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/standard.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/throttle_error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retry/timeout_error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/retryer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/runtime.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/cache.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/const.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/header_rules.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/headers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/hmac.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/host.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/scope.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/internal/v4/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/presign_middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/stream.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/signer/v4/v4.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/to_ptr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/content_type.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/response_error_middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/transport/http/timeout_read_closer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/types.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/aws/version.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/auth_scheme_preference.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/defaultsmode.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/env_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/generate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/errors.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/ini.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/parse.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/sections.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/token.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/tokenize.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/internal/ini/value.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/load_options.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/local.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_bearer_token.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/resolve_credentials.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/config/shared_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ec2rolecreds/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/auth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/endpoints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/internal/client/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/endpointcreds/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/dpop.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/file.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/logincreds/token.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/processcreds/provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_cached_token.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_credentials_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/ssocreds/sso_token_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/static_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/assume_role_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/credentials/stscreds/web_identity_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetDynamicData.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetIAMInfo.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetInstanceIdentityDocument.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetMetadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetRegion.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetToken.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/api_op_GetUserData.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/auth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/endpoints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/internal/config/resolvers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/request_middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/feature/ec2/imds/token_provider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/auth.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/scheme.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_adapter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/bearer_token_signer_adapter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/credentials_adapter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/smithy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/auth/smithy/v4signer_adapter.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/endpoints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/configsources/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/context/context.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/arn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/doc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/generate.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/host.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partition.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/awsrulesfn/partitions.jsonis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/endpoints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/endpoints.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/endpoints/v2/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/rand/rand.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/interfaces.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdk/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sdkio/byte.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/shareddefaults/shared_config.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/strings/strings.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/LICENSEis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/docs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/sync/singleflight/singleflight.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/timeconv/duration.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/credentials.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/error.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/go_module_metadata.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/compare.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/crypto/ecc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/const.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/header_rules.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/headers.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/hmac.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/host.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/time.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/internal/v4/util.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/presign_middleware.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/smithy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/internal/v4a/v4a.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/CHANGELOG.mdis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/LICENSE.txtis excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_client.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptAddressTransfer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptCapacityReservationBillingOwnership.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptReservedInstancesExchangeQuote.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayClientVpnAttachment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayMulticastDomainAssociations.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayPeeringAttachment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptTransitGatewayVpcAttachment.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptVpcEndpointConnections.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AcceptVpcPeeringConnection.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AdvertiseByoipCidr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateAddress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateHosts.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AllocateIpamPoolCidr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_ApplySecurityGroupsToClientVpnTargetNetwork.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignIpv6Addresses.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignPrivateIpAddresses.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssignPrivateNatGatewayAddress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateAddress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateCapacityReservationBillingOwner.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateClientVpnTargetNetwork.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateDhcpOptions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateEnclaveCertificateIamRole.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIamInstanceProfile.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateInstanceEventWindow.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIpamByoasn.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateIpamResourceDiscovery.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateNatGatewayAddress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateRouteServer.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateRouteTable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateSecurityGroupVpc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateSubnetCidrBlock.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTransitGatewayMulticastDomain.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTransitGatewayPolicyTable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTransitGatewayRouteTable.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateTrunkInterface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AssociateVpcCidrBlock.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachClassicLinkVpc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachInternetGateway.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachNetworkInterface.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachVerifiedAccessTrustProvider.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachVolume.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AttachVpnGateway.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AuthorizeClientVpnIngress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AuthorizeSecurityGroupEgress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_AuthorizeSecurityGroupIngress.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_BundleInstance.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelBundleTask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelCapacityReservation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelCapacityReservationFleets.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelConversionTask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelDeclarativePoliciesReport.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelExportTask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelImageLaunchPermission.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelImportTask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelReservedInstancesListing.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelSpotFleetRequests.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CancelSpotInstanceRequests.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_ConfirmProductInstance.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopyFpgaImage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopyImage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopySnapshot.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CopyVolumes.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityManagerDataExport.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityReservation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityReservationBySplitting.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCapacityReservationFleet.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCarrierGateway.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateClientVpnEndpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateClientVpnRoute.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCoipCidr.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCoipPool.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateCustomerGateway.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDefaultSubnet.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDefaultVpc.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDelegateMacVolumeOwnershipTask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateDhcpOptions.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateEgressOnlyInternetGateway.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateFleet.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateFlowLogs.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateFpgaImage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateImage.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateImageUsageReport.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInstanceConnectEndpoint.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInstanceEventWindow.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInstanceExportTask.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInternetGateway.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateInterruptibleCapacityReservationAllocation.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpam.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpamExternalResourceVerificationToken.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpamPolicy.gois excluded by!**/vendor/**,!vendor/**vendor/github.com/aws/aws-sdk-go-v2/service/ec2/api_op_CreateIpamPool.gois excluded by!**/vendor/**,!vendor/**
📒 Files selected for processing (8)
go.modpkg/dns/aws/dns.gopkg/dns/aws/dns_test.gopkg/util/aws/shared_credentials_file.gopkg/util/aws/shared_credentials_file_test.gotest/e2e/aws_util_test.gotest/e2e/lb_eip_test.gotest/e2e/operator_test.go
The aws-sdk-go library, used on DNS controller is deprecated and does not receive updates anymore. This change migrates to the new aws-sdk-go-v2, adding the new library and making the required changes to keep the code compatible
|
@coderabbitai I want you to do a full review, line by line of the changes of this PR (ignore the vendor directory!) trying to find critical or major issues, possible breaking changes, misbehaviors and mistakes that can cause issues to users. Be critical on it, as this is a very sensible change |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll read all the relevant changed files to perform the thorough review you requested. 🏁 Script executed: Length of output: 269260
🔴 Critical / Breaking Issues1. GovCloud detection change: C2S/SC2S now use Alias records instead of CNAMEFile: if isGovCloudRegion(m.route53.Options().Region) {
// CNAME path
} else {
// Alias A/AAAA path
}In v1, the helper Please verify whether the v1 2.
|
| # | Severity | File | Issue |
|---|---|---|---|
| 1 | 🔴 Critical | dns.go |
C2S/SC2S may now use Alias instead of CNAME (behavior change from v1) |
| 2 | 🔴 Critical | dns.go |
lookupZoneID returns error on first ARN parse failure without trying rest |
| 3 | 🔴 Critical | dns.go |
Empty SharedCredentialFile passed to SDK options without guard |
| 4 | 🟠 Major | dns.go + test |
resolveEndpoint is logging-only; tests don't verify actual SDK endpoints |
| 5 | 🟠 Major | dns.go |
GovCloud tagging URL override uses brittle substring matching |
| 6 | 🟠 Major | dns.go |
break in switch default for EUSC is non-idiomatic and confusing |
| 7 | 🟠 Major | dns_test.go |
Missing Test_NewProvider case for EUSC with default endpoints |
| 8 | 🟠 Major | dns.go |
No dual-stack (AAAA) support in GovCloud CNAME path |
| 9 | 🟡 Minor | lb_eip_test.go |
Deprecated rand.Seed |
| 10 | 🟡 Minor | operator_test.go |
Deprecated io/ioutil not cleaned up |
| 11 | 🟡 Minor | dns.go |
Pervasive context.TODO() prevents timeout/cancellation propagation |
| 12 | 🟡 Minor | dns.go |
resolveEndpoint silently returns "" for tagging in iso partitions |
| 13 | 🟡 Minor | dns.go |
Shallow copy of cfg into cfgRoute53 — intent should be commented |
The most important item to verify before merging is Issue #1 (the GovCloud detection change affecting C2S/SC2S). The rest are important quality/robustness issues but lower risk for standard AWS deployments.
|
@coderabbitai about your finding 1, The v1 Route53 endpoint for us-iso-east-1 would have been https://route53.c2s.ic.gov — which does NOT contain "us-gov". So clientEndpointIsGovCloud returned false for C2S/SC2S |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 988 🏁 Script executed: Length of output: 741 🏁 Script executed: Length of output: 392
govCloudRoute53Region = "us-gov" // line 46
func clientEndpointIsGovCloud(clientInfo *metadata.ClientInfo) bool {
return strings.Contains(clientInfo.Endpoint, govCloudRoute53Region)
}The v1 function checked whether the Route53 endpoint URL contained The v2 ✏️ Learnings added
|
|
@coderabbitai some more assessment from your comments: comment on all of them and if so, do another review line by line checking if all of your concerns are addressed |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 71906
Acknowledged Invalid Findings (with v1 evidence)#2 — #3 — Empty #4 — Tests don't verify actual SDK endpoints: Confirmed. In v2 there is no #5 — GovCloud tagging URL substring match: Confirmed. v1 lines 247–250 contain the identical #6 — #8 — No dual-stack in GovCloud CNAME path: Confirmed. v1 #12 — #13 (self-correction) — Fresh Line-by-Line Re-ReviewWith all 8 invalidated findings set aside, here is a clean pass on what genuinely remains. 🟠 Remaining Major Issue#7 — No The test file covers EUSC only with custom service endpoints: {
name: "custom service endpoints, EUSC Brandenburg",
config: Config{
Region: "eusc-de-east-1",
ServiceEndpoints: []ServiceEndpoint{...},
},
...
}There is no test case for {
name: "default service endpoints, EUSC Brandenburg",
config: Config{
Region: "eusc-de-east-1",
},
expectedTaggingServiceEndpoint: "https://tagging.eusc-de-east-1.amazonaws.eu",
expectedElbServiceEndpointEndpoint: "https://elasticloadbalancing.eusc-de-east-1.amazonaws.eu",
expectedElbv2ServiceEndpointEndpoint: "https://elasticloadbalancing.eusc-de-east-1.amazonaws.eu",
expectedRoute53ServiceEndpoint: "https://route53.amazonaws.eu",
},🟡 Remaining Minor Issues#9 — No-op rand.Seed(time.Now().UnixNano())This uses #10 — Deprecated Lines 992, 2473, 2615, 2887, 3260, and 3333 still use #11 — Pervasive All AWS API calls in Summary of Remaining Items
No new critical or breaking issues found in the fresh pass. The migration is behaviorally faithful to v1. The only concrete code suggestion I'd flag before merging is #7 (EUSC default-endpoint test case). ✏️ Learnings added
|
|
@coderabbitai do the same review I have asked you again, line by line, critical and major findings, adversarial (give me reasons not to merge, if you have some). Also verify why ci/prow/e2e-aws-operator execution failed before, and if it is related with this PR |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainI'll do the fresh adversarial re-review and simultaneously investigate the CI failure. 🏁 Script executed: Length of output: 81698 All CI checks are currently PENDING (re-triggered just now at ~12:01 UTC). Let me check what the previous failure was, and simultaneously verify the Let me check the prow job history for the previous failed run. 🏁 Script executed: Length of output: 8730 Found the previous failed run. Let me fetch its logs to diagnose the failure. Let me fetch the specific prow job logs directly. 🏁 Script executed: Length of output: 2737 Good - confirmed FAILURE on the previous commit. Now let me get the build log and test results. 🏁 Script executed: Length of output: 22299 Found the failing test: 🏁 Script executed: Length of output: 12999 Let me get the detailed test artifacts for the specific failure. 🏁 Script executed: Length of output: 33121
Fresh Adversarial Re-Review✅ Correction from previous round: Finding
|
| Failed test | TestManagedDNSToUnmanagedDNSIngressController |
| Error | DNSRecord zone expected to have status=Unknown but got status=True |
| Path exercised | DNS Delete path (managed→unmanaged transition) |
| PR-related? | Suspicious — this test exclusively exercises the DNS delete path which this PR rewrites, and the inverse test (unmanaged→managed, DNS create) passed. |
| Verdict | Do not merge until the re-triggered CI run passes. If it fails again, instrument the Delete/updateRecord code path with more diagnostic logging and verify the error detection logic (Finding #2 above). |
✏️ Learnings added
Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445
Timestamp: 2026-05-14T12:10:08.890Z
Learning: In the cluster-ingress-operator (pkg/dns/aws/dns.go) v2 migration PR `#1445`, the `TestManagedDNSToUnmanagedDNSIngressController` e2e test failed on a previous commit (6d8c121f) with error "DNSRecord zone expected to have status=Unknown but got status=True" - meaning after switching to UnmanagedLoadBalancerDNS, the internal zone's DNSRecord condition was still True instead of Unknown. This test exercises the DNS DELETE/disownment path which was rewritten in the v2 migration.
Learnt from: rikatz
URL: https://github.com/openshift/cluster-ingress-operator/pull/1445
Timestamp: 2026-05-14T12:10:08.890Z
Learning: In cluster-ingress-operator pkg/dns/aws/dns.go (PR `#1445`), Test_NewProvider does include a "default service endpoints, EUSC Brandenburg" test case with no ServiceEndpoints configured - verifying default endpoint resolution for eusc-de-east-1. Finding that this test was missing was incorrect.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
The test that I care right now is e2e-aws-operator, which is flaky by different reasons apparently, so let me try again /test e2e-aws-operator |
|
@rikatz: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
The aws-sdk-go library, used on DNS controller is deprecated and does not receive updates anymore.
This change migrates to the new aws-sdk-go-v2, adding the new library and making the required changes to keep the code compatible