Skip to content

[AV-129893] Revert: Enhance network peer error handling and state management" #621

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.

Sign up for GitHub

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

Merged
Talina06 merged 1 commit into from
May 29, 2026
Merged

[AV-129893] Revert: Enhance network peer error handling and state management" #621

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 33 additions & 98 deletions internal/resources/network_peer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
"net/http"
"strings"

"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-log/tflog"

"github.com/couchbasecloud/terraform-provider-couchbase-capella/internal/api"
network_peer_api "github.com/couchbasecloud/terraform-provider-couchbase-capella/internal/api/network_peer"
"github.com/couchbasecloud/terraform-provider-couchbase-capella/internal/errors"
providerschema "github.com/couchbasecloud/terraform-provider-couchbase-capella/internal/schema"

"github.com/hashicorp/terraform-plugin-framework/path"
"github.com/hashicorp/terraform-plugin-framework/resource"
"github.com/hashicorp/terraform-plugin-framework/types"
"github.com/hashicorp/terraform-plugin-log/tflog"
)

// Ensure the implementation satisfies the expected interfaces.
Expand Down Expand Up @@ -129,38 +129,10 @@
nil,
)
if err != nil {
// The API may persist a failed peering record even when returning a non-success status.
// Attempt to find the resource so we can save it to state for lifecycle management.
peerID := n.findNetworkPeerByName(ctx, organizationId, projectId, clusterId, plan.Name.ValueString())
if peerID == "" {
resp.Diagnostics.AddError(
"Error creating network peer",
errorMessageWhileNetworkPeerCreation+api.ParseError(err),
)
return
}

tflog.Info(ctx, "network peer was persisted despite creation error, saving to state", map[string]interface{}{
"peer_id": peerID,
})

resp.Diagnostics.AddWarning(
"Network peer created with errors",
fmt.Sprintf(
"The API returned an error during creation (%s) but the network peer %q was persisted. "+
"The resource has been saved to state for lifecycle management. "+
"Review the peer status in Capella and, if necessary, run terraform destroy to clean up.",
api.ParseError(err), peerID,
),
resp.Diagnostics.AddError(
"Error creating network peer",
errorMessageWhileNetworkPeerCreation+api.ParseError(err),
)

diags = resp.State.Set(ctx, initializeNetworkPeerPlanId(plan, peerID))
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}

n.setRefreshedStateOrWarn(ctx, resp, organizationId, projectId, clusterId, peerID, plan.ProviderType.ValueString(), err)
return
}

Expand All @@ -169,7 +141,7 @@
if err != nil {
resp.Diagnostics.AddError(
"Error creating network peer",
errorMessageWhileNetworkPeerCreation+"error during unmarshalling: "+err.Error(),
errorMessageWhileNetworkPeerCreation+"error during unmarshalling:"+err.Error(),
)
return
}
Expand All @@ -180,28 +152,21 @@
return
}

n.setRefreshedStateOrWarn(ctx, resp, organizationId, projectId, clusterId, networkPeerResponse.Id.String(), plan.ProviderType.ValueString(), nil)
}

// setRefreshedStateOrWarn attempts to read the full peer state and set it on the response.
// If the read fails (e.g. peer is in failed state with incomplete providerConfig), a warning
// is emitted instead of an error so that the partial state (containing the ID) is preserved.
func (n *NetworkPeer) setRefreshedStateOrWarn(ctx context.Context, resp *resource.CreateResponse, organizationId, projectId, clusterId, peerID, providerType string, originalErr error) {
refreshedState, err := n.retrieveNetworkPeer(ctx, organizationId, projectId, clusterId, peerID, providerType)
refreshedState, err := n.retrieveNetworkPeer(ctx, organizationId, projectId, clusterId, networkPeerResponse.Id.String(), plan.ProviderType.ValueString())
if err != nil {
detail := "The network peer was created but its current status could not be fully read: " + err.Error()
if originalErr != nil {
detail += ". Original creation error: " + api.ParseError(originalErr)
}
resp.Diagnostics.AddWarning(
"Network peer created with incomplete state",
detail,
resp.Diagnostics.AddError(
"Error reading network peering service status",
"Error reading network peering service status, unexpected error: "+err.Error(),
)

return
Comment thread
Talina06 marked this conversation as resolved.
}

diags := resp.State.Set(ctx, refreshedState)
diags = resp.State.Set(ctx, refreshedState)
resp.Diagnostics.Append(diags...)
if resp.Diagnostics.HasError() {
return
}
}

// createAWSProviderConfig is the function to handle AWS configuration.
Expand Down Expand Up @@ -458,15 +423,7 @@
) (*providerschema.NetworkPeer, error) {
networkPeerResp, err := n.getNetworkPeer(ctx, organizationId, projectId, clusterId, peerId)
if err != nil {
return nil, fmt.Errorf("%w: %w", errors.ErrNotFound, err)
}

// Use the known provider type when the response doesn't include it
// (common for failed peers with empty providerConfig).
if networkPeerResp.ProviderType == "" {
networkPeerResp.ProviderType = providerType
} else if validateProviderTypeIsSameInPlanAndState(providerType, networkPeerResp.ProviderType) {
networkPeerResp.ProviderType = providerType
return nil, fmt.Errorf("%s: %w", errors.ErrNotFound, err)

Check failure on line 426 in internal/resources/network_peer.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
}

audit := providerschema.NewCouchbaseAuditData(networkPeerResp.Audit)
Expand All @@ -476,6 +433,10 @@
return nil, fmt.Errorf("%s: %w", errors.ErrUnableToConvertAuditData, err)
}

if validateProviderTypeIsSameInPlanAndState(providerType, networkPeerResp.ProviderType) {
networkPeerResp.ProviderType = providerType
}

refreshedState, err := providerschema.NewNetworkPeer(ctx, networkPeerResp, organizationId, projectId, clusterId, auditObj)
if err != nil {
return nil, fmt.Errorf("%s: %w", errors.ErrRefreshingState, err)
Expand All @@ -486,8 +447,8 @@
return refreshedState, nil
}

// getNetworkPeer retrieves network peer information from the specified organization and project
// using the provided cluster ID and peer ID by open-api call.
// getNetworkPeer retrieves cluster information from the specified organization and project
// using the provided cluster ID by open-api call.
Comment thread
Talina06 marked this conversation as resolved.
func (n *NetworkPeer) getNetworkPeer(
ctx context.Context, organizationId, projectId, clusterId, peerId string,
) (*network_peer_api.GetNetworkPeeringRecordResponse, error) {
Expand All @@ -510,21 +471,16 @@
return nil, fmt.Errorf("%s: %w", errors.ErrUnmarshallingResponse, err)
}

// Best-effort provider type detection from response config fields.
// For failed peers, providerConfig may be empty so this detection is not fatal.
_ = defineProviderForResponse(&networkResp)
if err := defineProviderForResponse(networkResp); err != nil {
return nil, err
}
Comment thread
Talina06 marked this conversation as resolved.

return &networkResp, nil
}

// defineProviderForResponse sets the provider type on the response based on populated
// provider config fields. Returns an error if provider type cannot be determined (e.g.
// when a failed peer has an empty providerConfig), but callers may choose to ignore it.
func defineProviderForResponse(networkResp *network_peer_api.GetNetworkPeeringRecordResponse) error {
if len(networkResp.ProviderConfig) == 0 || string(networkResp.ProviderConfig) == "null" {
return fmt.Errorf("%w: providerConfig is empty", errors.ErrReadingProviderConfig)
}

// defineProviderForResponse sets the provider type in the retrieved network peer as per the fields populated in the provider config.
// If the provider type is not set through terraform separately in this manner, it will throw error as v4 get doesn't return it, but it's a field in resources.
func defineProviderForResponse(networkResp network_peer_api.GetNetworkPeeringRecordResponse) error {
azure, err := networkResp.AsAZURE()
if err != nil {
return fmt.Errorf("%s: %w", errors.ErrReadingAzureConfig, err)
Expand All @@ -540,6 +496,7 @@
return fmt.Errorf("%s: %w", errors.ErrReadingAWSConfig, err)
}

// if there is no error, set the provider type for the provider config as per the populated fields in the get response.
switch {
case azure.AzureConfigData.AzureTenantId != "":
networkResp.ProviderType = "azure"
Expand All @@ -548,34 +505,12 @@
case aws.AWSConfigData.VpcId != "":
networkResp.ProviderType = "aws"
default:
return fmt.Errorf("%w: unable to determine provider type from config fields", errors.ErrReadingProviderConfig)
return fmt.Errorf("%s: %w", errors.ErrReadingProviderConfig, err)

Check failure on line 508 in internal/resources/network_peer.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
Comment thread
Talina06 marked this conversation as resolved.
}
Comment thread
Talina06 marked this conversation as resolved.

return nil
}

// findNetworkPeerByName lists all network peers for a cluster and returns the ID
// of the peer whose name matches. Returns empty string if not found.
func (n *NetworkPeer) findNetworkPeerByName(ctx context.Context, organizationId, projectId, clusterId, name string) string {
url := fmt.Sprintf("%s/v4/organizations/%s/projects/%s/clusters/%s/networkPeers", n.HostURL, organizationId, projectId, clusterId)
cfg := api.EndpointCfg{Url: url, Method: http.MethodGet, SuccessStatus: http.StatusOK}

peers, err := api.GetPaginated[[]network_peer_api.GetNetworkPeeringRecordResponse](ctx, n.ClientV1, n.Token, cfg, api.SortById)
if err != nil {
tflog.Warn(ctx, "failed to list network peers while searching for persisted record", map[string]interface{}{
"error": err.Error(),
})
return ""
}

for i := range peers {
if peers[i].Name == name {
return peers[i].Id.String()
}
}
return ""
}

func validateProviderTypeIsSameInPlanAndState(planProviderType, stateProviderType string) bool {
return strings.EqualFold(planProviderType, stateProviderType)
}
37 changes: 10 additions & 27 deletions internal/schema/network_peer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,16 +210,13 @@

newProviderConfig, err := morphToProviderConfig(networkPeer)
if err != nil {
return nil, fmt.Errorf("%w: %w", errors.ErrConvertingProviderConfig, err)
return nil, fmt.Errorf("%s: %w", errors.ErrConvertingProviderConfig, err)

Check failure on line 213 in internal/schema/network_peer.go

View workflow job for this annotation

GitHub Actions / golangci-lint 

non-wrapping format verb for fmt.Errorf. Use `%w` to format errors (errorlint)
}
newNetworkPeer.ProviderConfig = &newProviderConfig

if networkPeer.Status.State != nil {
state := *networkPeer.Status.State
var reasoning string
if networkPeer.Status.Reasoning != nil {
reasoning = *networkPeer.Status.Reasoning
}
reasoning := *networkPeer.Status.Reasoning
Comment thread
Talina06 marked this conversation as resolved.
status := PeeringStatus{
State: types.StringValue(state),
Reasoning: types.StringValue(reasoning),
Expand All @@ -233,7 +230,7 @@

newCommands, err := MorphCommands(networkPeer.Commands)
if err != nil {
return nil, fmt.Errorf("%w: %w", errors.ErrConvertingCidr, err)
return nil, fmt.Errorf("%s: %w", errors.ErrConvertingCidr, err)
}

newNetworkPeer.Commands = newCommands
Expand All @@ -242,28 +239,22 @@
}

// morphToProviderConfig is used to convert ProviderConfig from json.RawMessage format to ProviderConfig type.
// Returns an empty ProviderConfig with no error when the providerConfig field is null or empty
// (which occurs for network peers in a failed state).
func morphToProviderConfig(networkPeer *network_peer_api.GetNetworkPeeringRecordResponse) (ProviderConfig, error) {
var newProviderConfig ProviderConfig

if len(networkPeer.ProviderConfig) == 0 || string(networkPeer.ProviderConfig) == "null" {
return newProviderConfig, nil
}

aws, err := networkPeer.AsAWS()
Comment thread
Talina06 marked this conversation as resolved.
if err != nil {
return ProviderConfig{}, fmt.Errorf("%w: %w", errors.ErrReadingAWSConfig, err)
return ProviderConfig{}, fmt.Errorf("%s: %w", errors.ErrReadingAWSConfig, err)
}
Comment thread
Talina06 marked this conversation as resolved.

gcp, err := networkPeer.AsGCP()
if err != nil {
return ProviderConfig{}, fmt.Errorf("%w: %w", errors.ErrReadingGCPConfig, err)
return ProviderConfig{}, fmt.Errorf("%s: %w", errors.ErrReadingGCPConfig, err)
}

azure, err := networkPeer.AsAZURE()
if err != nil {
return ProviderConfig{}, fmt.Errorf("%w: %w", errors.ErrReadingAzureConfig, err)
return ProviderConfig{}, fmt.Errorf("%s: %w", errors.ErrReadingAzureConfig, err)
}

switch {
Expand Down Expand Up @@ -296,7 +287,7 @@
}
return newProviderConfig, nil
default:
return newProviderConfig, nil
return ProviderConfig{}, fmt.Errorf("%s: %w", errors.ErrReadingProviderConfig, err)
Comment thread
Talina06 marked this conversation as resolved.
}

}
Expand Down Expand Up @@ -328,27 +319,19 @@

// NewNetworkPeerData create new network peer data object.
func NewNetworkPeerData(networkPeer *network_peer_api.GetNetworkPeeringRecordResponse, organizationId, projectId, clusterId string, auditObject basetypes.ObjectValue) (*NetworkPeerData, error) {
var state, reasoning string
if networkPeer.Status.State != nil {
state = *networkPeer.Status.State
}
if networkPeer.Status.Reasoning != nil {
reasoning = *networkPeer.Status.Reasoning
}

newNetworkPeerData := NetworkPeerData{
Id: types.StringValue(networkPeer.Id.String()),
Name: types.StringValue(networkPeer.Name),
Audit: auditObject,
Status: PeeringStatus{
State: types.StringValue(state),
Reasoning: types.StringValue(reasoning),
State: types.StringValue(*networkPeer.Status.State),
Comment thread
Talina06 marked this conversation as resolved.
Reasoning: types.StringValue(*networkPeer.Status.Reasoning),
},
Comment thread
Talina06 marked this conversation as resolved.
}

newProviderConfig, err := morphToProviderConfig(networkPeer)
if err != nil {
return nil, fmt.Errorf("%w: %w", errors.ErrConvertingProviderConfig, err)
return nil, fmt.Errorf("%s: %w", errors.ErrConvertingProviderConfig, err)
}
newNetworkPeerData.ProviderConfig = newProviderConfig

Expand Down
Loading
Loading