Skip to content

Conversation

@fabiencastarede
Copy link
Contributor

@fabiencastarede fabiencastarede commented Dec 19, 2025

Fix: Handle empty external-name in GetIDFn to prevent incomplete composite IDs

Description

This PR fixes an issue where resources with composite ID formats were generating incomplete IDs in Terraform state when the external-name was empty (during initial resource creation). This caused Terraform refresh operations to fail with OVH API errors.

Problem

When resources are being created, they don't yet have an external-name annotation set. However, the GetIDFn function was still being called during state initialization, causing it to construct incomplete composite IDs such as:

  • f93f1228ddd841578fc3069b5cf2fd7d/ (missing user_id for User resource)
  • f93f1228ddd841578fc3069b5cf2fd7d/EU-WEST-PAR/ (missing gateway_id for Gateway resource)
  • f93f1228ddd841578fc3069b5cf2fd7d/network-id/ (missing subnet_id for Subnet resource)

These incomplete IDs were then set in the Terraform state, causing subsequent refresh operations to fail with OVH API errors:

Error: Given data (f93f1228ddd841578fc3069b5cf2fd7d/EU-WEST-PAR/) is not valid for type uuid
Error: expected 'userId' to be of type int

Root Cause

The issue occurs in the following flow:

  1. Resource creation starts with empty external-name
  2. EnsureTFState() is called to initialize Terraform state
  3. GetIDFn() is called with empty externalName parameter
  4. GetIDFn() constructs partial composite ID (e.g., service_name/)
  5. Incomplete ID is set in tfstate
  6. Terraform refresh tries to query OVH API with malformed ID
  7. OVH API rejects the request

Solution

Updated GetIDFn implementations for resources with composite ID formats to check if externalName is empty and return an empty string instead of constructing an incomplete ID.

Changes

Modified config/external_name.go to add empty externalName checks in GetIDFn for:

  1. PrivateNetwork (ovh_cloud_project_network_private)

    • Format: service_name/network_id
    • Check: Return "" if externalName is empty
  2. Subnet (ovh_cloud_project_network_private_subnet)

    • Format: service_name/network_id/subnet_id
    • Check: Return "" if externalName is empty
  3. User (ovh_cloud_project_user)

    • Format: service_name/user_id
    • Check: Return "" if externalName is empty
  4. S3Credentials (ovh_cloud_project_user_s3_credential)

    • Format: service_name/user_id/access_key_id
    • Check: Return "" if externalName is empty
  5. Gateway (ovh_cloud_project_gateway)

    • Format: service_name/region/gateway_id
    • Check: Return "" if externalName is empty
  6. S3Policy (ovh_cloud_project_user_s3_policy)

    • Format: service_name/user_id
    • Special case: Doesn't use externalName in ID, only added documentation comment

Test Scenarios

Scenario Result
Create new PrivateNetwork ✅ No refresh errors
Create new User ✅ No refresh errors
Create new Gateway ✅ No refresh errors
Create new S3Credentials ✅ No refresh errors
Create new Subnet ✅ No refresh errors
Import existing resources ✅ Proper composite IDs used
Pod restart (async resources) ✅ Import works correctly

Impact

  • Severity: Bug fix for resource creation
  • Scope: Affects 5 resources with composite ID formats
  • Risk: Low - only changes behavior when externalName is empty
  • Breaking Changes: None
  • Performance: No impact

Related Work

Notes

This fix is essential for resources that use composite IDs (IDs composed of multiple parameters). When combined with the upjet Import fallback for async resources, it provides a complete solution for handling resource state across pod restarts and Velero backup/restore operations.

The S3Policy resource configuration was updated with documentation comments but doesn't require the empty check since its ID format (service_name/user_id) doesn't depend on the externalName value.

Fix import ID format for resources that require composite IDs:
- PrivateNetwork: service_name/network_id
- Subnet: service_name/network_id/subnet_id
- User: service_name/user_id
- S3Credentials: service_name/user_id/access_key_id
- S3Policy: service_name/user_id
- Gateway: service_name/region/gateway_id

This fixes import failures for async resources after pod restart
when using the Import fallback mechanism.
This fix prevents incomplete composite IDs from being set in Terraform
state when resources are being created (before external-name is assigned).

Problem:
--------
When resources were being created with empty external-name, GetIDFn was
constructing incomplete IDs like:
- service_name/ (missing resource_id)
- service_name/region/ (missing gateway_id)

This caused Terraform refresh to fail with OVH API errors:
- "Given data (...) is not valid for type uuid"
- "expected 'userId' to be of type int"

Solution:
---------
Updated GetIDFn for the following resources to check if externalName
is empty and return "" instead of constructing partial IDs:
- PrivateNetwork (service_name/network_id)
- Subnet (service_name/network_id/subnet_id)
- User (service_name/user_id)
- S3Credentials (service_name/user_id/access_key_id)
- Gateway (service_name/region/gateway_id)

S3Policy kept its existing behavior since it doesn't use external-name
in its ID format (only service_name/user_id).

Testing:
--------
- ✅ Resources create successfully without refresh errors
- ✅ Import works correctly with proper composite IDs
- ✅ No incomplete IDs set in tfstate during creation

Signed-off-by: Fabien Castarède <[email protected]>
@smileisak
Copy link
Member

@fabiencastarede thank you for this PR, LGTM. Could you meanwhile fix the local-deploy job please?

@fabiencastarede
Copy link
Contributor Author

fabiencastarede commented Dec 22, 2025

@smileisak I'm not sure about what's wrong with the local-deploy job: the Dockerfile has not changed and the error log mentioned a 502 bad gateway:

ERROR: failed to copy: httpReadSeeker: failed open: unexpected status code https://registry-1.docker.io/v2/library/alpine/blobs/sha256:042a816809aac8d0f7d7cacac7965782ee2ecac3f21bcf9f24b1de1a7387b769: 502 Bad Gateway

Job runs without error on my machine.
Maybe just retry the job?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants