Skip to content

fix(all): private_ip(s) should only be set when no error occurred #3094

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
merged 3 commits into from
May 22, 2025
Merged

fix(all): private_ip(s) should only be set when no error occurred #3094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ec573b
private_ips as optional attribute, only set it when no error occurred
Mia-Cross May 19, 2025
233ad94
Merge branch 'master' into fix_read_private_ips_when_unauthorized
Mia-Cross May 21, 2025
6b8a50d
Merge branch 'master' into fix_read_private_ips_when_unauthorized
Mia-Cross May 22, 2025
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
26 changes: 16 additions & 10 deletions internal/services/baremetal/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,7 @@ If this behaviour is wanted, please set 'reinstall_on_ssh_key_changes' argument
"private_ips": {
Type: schema.TypeList,
Computed: true,
Optional: true,
Description: "List of private IPv4 and IPv6 addresses associated with the resource",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -534,6 +535,7 @@ func ResourceServerRead(ctx context.Context, d *schema.ResourceData, m interface
privateNetworkIDs = append(privateNetworkIDs, pn.PrivateNetworkID)
}

// Read private IPs if possible
allPrivateIPs := make([]map[string]interface{}, 0, listPrivateNetworks.TotalCount)
diags := diag.Diagnostics{}

Expand All @@ -542,25 +544,29 @@ func ResourceServerRead(ctx context.Context, d *schema.ResourceData, m interface
opts := &ipam.GetResourcePrivateIPsOptions{
ResourceType: &resourceType,
PrivateNetworkID: &privateNetworkID,
ProjectID: &server.ProjectID,
}

privateIPs, err := ipam.GetResourcePrivateIPs(ctx, m, pnRegion, opts)
if err != nil {
if !httperrors.Is403(err) {
return diag.FromErr(err)
}

switch {
case err == nil:
allPrivateIPs = append(allPrivateIPs, privateIPs...)
case httperrors.Is403(err):
return append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Unauthorized to read server's private IPs, please check your IAM permissions",
Detail: err.Error(),
AttributePath: cty.GetAttrPath("private_ips"),
})
default:
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: err.Error(),
Detail: "Got 403 while reading private IPs from IPAM API, please check your IAM permissions",
Summary: fmt.Sprintf("Unable to get private IPs for server %s (pn_id: %s)", server.ID, privateNetworkID),
Detail: err.Error(),
AttributePath: cty.GetAttrPath("private_ips"),
})
}

if privateIPs != nil {
allPrivateIPs = append(allPrivateIPs, privateIPs...)
}
}

_ = d.Set("private_ips", allPrivateIPs)
Expand Down
1,293 changes: 671 additions & 622 deletions internal/services/baremetal/testdata/server-create-server-with-private-network.cassette.yaml
View file
Open in desktop

Large diffs are not rendered by default.

16 changes: 16 additions & 0 deletions internal/services/instance/helpers_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,3 +536,19 @@ func prepareRootVolume(rootVolumeI map[string]any, serverType *instance.ServerTy
Boot: rootVolumeIsBootVolume,
}
}

func getServerProjectID(ctx context.Context, api *instance.API, zone scw.Zone, serverID string) (string, error) {
server, err := api.GetServer(&instance.GetServerRequest{
Zone: zone,
ServerID: serverID,
}, scw.WithContext(ctx))
if err != nil {
return "", fmt.Errorf("get private NIC's project ID: error getting server %s", serverID)
}

if server.Server.Project == "" {
return "", fmt.Errorf("no project ID found for server %s", serverID)
}

return server.Server.Project, nil
}
43 changes: 33 additions & 10 deletions internal/services/instance/private_nic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package instance

import (
"context"
"fmt"

"github.com/hashicorp/go-cty/cty"
"github.com/hashicorp/terraform-plugin-sdk/v2/diag"
Expand Down Expand Up @@ -74,6 +75,7 @@ func ResourcePrivateNIC() *schema.Resource {
"private_ips": {
Type: schema.TypeList,
Computed: true,
Optional: true,
Description: "List of private IPv4 and IPv6 addresses associated with the resource",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -185,35 +187,56 @@ func ResourceInstancePrivateNICRead(ctx context.Context, d *schema.ResourceData,
_ = d.Set("tags", privateNIC.Tags)
}

// Get private NIC's private IPs if possible
diags := diag.Diagnostics{}

region, err := zone.Region()
if err != nil {
return diag.FromErr(err)
return append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Unable to get private NIC's private IPs",
Detail: err.Error(),
})
}

projectID, err := getServerProjectID(ctx, instanceAPI, zone, privateNIC.ServerID)
if err != nil {
return append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Unable to get private NIC's private IPs",
Detail: err.Error(),
})
}

diags := diag.Diagnostics{}
resourceType := ipamAPI.ResourceTypeInstancePrivateNic
opts := &ipam.GetResourcePrivateIPsOptions{
ResourceID: &privateNIC.ID,
ResourceType: &resourceType,
PrivateNetworkID: &privateNIC.PrivateNetworkID,
ProjectID: &projectID,
}

privateIPs, err := ipam.GetResourcePrivateIPs(ctx, m, region, opts)
if err != nil {
if !httperrors.Is403(err) {
return diag.FromErr(err)
}

switch {
case err == nil:
_ = d.Set("private_ips", privateIPs)
case httperrors.Is403(err):
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: err.Error(),
Detail: "Got 403 while reading private IPs from IPAM API, please check your IAM permissions",
Summary: "Unauthorized to read private NIC's private IPs, please check your IAM permissions",
Detail: err.Error(),
AttributePath: cty.GetAttrPath("private_ips"),
})
default:
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: fmt.Sprintf("Unable to get private IPs for pnic %s (server_id: %s)", privateNIC.ID, privateNIC.ServerID),
Detail: err.Error(),
AttributePath: cty.GetAttrPath("private_ips"),
})
}

_ = d.Set("private_ips", privateIPs)

return diags
}

Expand Down
101 changes: 56 additions & 45 deletions internal/services/instance/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ func ResourceServer() *schema.Resource {
"private_ips": {
Type: schema.TypeList,
Computed: true,
Optional: true,
Description: "List of private IPv4 addresses associated with the resource",
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
Expand Down Expand Up @@ -780,6 +781,41 @@ func ResourceInstanceServerRead(ctx context.Context, d *schema.ResourceData, m i

_ = d.Set("user_data", userData)

////
// Display warning if server will soon reach End of Service
////
diags := diag.Diagnostics{}

if server.EndOfService {
compatibleTypes, err := api.GetServerCompatibleTypes(&instanceSDK.GetServerCompatibleTypesRequest{
Zone: zone,
ServerID: id,
}, scw.WithContext(ctx))
if err != nil {
return diag.FromErr(err)
}

mostRelevantTypes := compatibleTypes.CompatibleTypes[:5]

diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Detail: fmt.Sprintf("Instance type %q will soon reach End of Service", server.CommercialType),
Summary: fmt.Sprintf(`Your Instance will soon reach End of Service. You can check the exact date on the Scaleway console. We recommend that you migrate your Instance before that.
Here are the %d best options for %q, ordered by relevance: [%s]

You can check the full list of compatible server types:
- on the Scaleway console
- using the CLI command 'scw instance server get-compatible-types %s zone=%s'`,
len(mostRelevantTypes),
server.CommercialType,
strings.Join(mostRelevantTypes, ", "),
server.ID,
server.Zone,
),
AttributePath: cty.GetAttrPath("type"),
})
}

////
// Read server private networks
////
Expand All @@ -799,75 +835,50 @@ func ResourceInstanceServerRead(ctx context.Context, d *schema.ResourceData, m i
privateNICIDs = append(privateNICIDs, nic.ID)
}

// Read server's private IPs if possible
allPrivateIPs := []map[string]interface{}(nil)
diags := diag.Diagnostics{}
resourceType := ipamAPI.ResourceTypeInstancePrivateNic

region, err := zone.Region()
if err != nil {
return diag.FromErr(err)
return append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Unable to get server's private IPs",
Detail: err.Error(),
})
}

for _, nicID := range privateNICIDs {
opts := &ipam.GetResourcePrivateIPsOptions{
ResourceType: &resourceType,
ResourceID: &nicID,
ProjectID: &server.Project,
}

privateIPs, err := ipam.GetResourcePrivateIPs(ctx, m, region, opts)
if err != nil {
if !httperrors.Is403(err) {
return diag.FromErr(err)
}

switch {
case err == nil:
allPrivateIPs = append(allPrivateIPs, privateIPs...)
case httperrors.Is403(err):
return append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: "Unauthorized to read server's private IPs, please check your IAM permissions",
Detail: err.Error(),
AttributePath: cty.GetAttrPath("private_ips"),
})
default:
diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Summary: err.Error(),
Detail: "Got 403 while reading private IPs from IPAM API, please check your IAM permissions",
Summary: fmt.Sprintf("Unable to get private IPs for server %s (pnic_id: %s)", server.ID, nicID),
Detail: err.Error(),
AttributePath: cty.GetAttrPath("private_ips"),
})
}

if privateIPs != nil {
allPrivateIPs = append(allPrivateIPs, privateIPs...)
}
}

_ = d.Set("private_ips", allPrivateIPs)

////
// Display warning if server will soon reach End of Service
////
if server.EndOfService {
compatibleTypes, err := api.GetServerCompatibleTypes(&instanceSDK.GetServerCompatibleTypesRequest{
Zone: zone,
ServerID: id,
}, scw.WithContext(ctx))
if err != nil {
return diag.FromErr(err)
}

mostRelevantTypes := compatibleTypes.CompatibleTypes[:5]

diags = append(diags, diag.Diagnostic{
Severity: diag.Warning,
Detail: fmt.Sprintf("Instance type %q will soon reach End of Service", server.CommercialType),
Summary: fmt.Sprintf(`Your Instance will soon reach End of Service. You can check the exact date on the Scaleway console. We recommend that you migrate your Instance before that.
Here are the %d best options for %q, ordered by relevance: [%s]

You can check the full list of compatible server types:
- on the Scaleway console
- using the CLI command 'scw instance server get-compatible-types %s zone=%s'`,
len(mostRelevantTypes),
server.CommercialType,
strings.Join(mostRelevantTypes, ", "),
server.ID,
server.Zone,
),
AttributePath: cty.GetAttrPath("type"),
})
}

return diags
}

Expand Down
3,017 changes: 1,974 additions & 1,043 deletions internal/services/instance/testdata/data-source-private-nic-basic.cassette.yaml
View file
Open in desktop

Large diffs are not rendered by default.

889 changes: 469 additions & 420 deletions internal/services/instance/testdata/private-nic-basic.cassette.yaml
View file
Open in desktop

Large diffs are not rendered by default.

1,676 changes: 1,034 additions & 642 deletions internal/services/instance/testdata/private-nic-tags.cassette.yaml
View file
Open in desktop

Large diffs are not rendered by default.

1,267 changes: 707 additions & 560 deletions internal/services/instance/testdata/private-nic-with-ipam.cassette.yaml
View file
Open in desktop

Large diffs are not rendered by default.

Loading
Loading