Skip to content

Conversation

@dguido
Copy link
Member

@dguido dguido commented Oct 27, 2025

Summary

Fixes #14846 - Resolves Scaleway deployment failure caused by broken scaleway_organization_info Ansible module.

Problem

The scaleway_organization_info module from ansible-collections/community.general is broken and returns empty data ([]) when called. This is due to the module using a deprecated API endpoint. See upstream issue: ansible-collections/community.general#3782

When users attempted to deploy to Scaleway, they encountered:

[ERROR]: Task failed: Module failed: 'NoneType' object has no attribute 'get'
Origin: /roles/cloud-scaleway/tasks/main.yml:6:7

This occurred because the role tried to access scaleway_org.scaleway_organization_info[0]['id'] on an empty list.

Solution

Instead of relying on the broken Ansible module, this PR:

  1. Prompts users for their Organization/Project ID - Added a clear prompt with instructions on where to find this in the Scaleway console
  2. Uses Scaleway Marketplace API - Replaces broken scaleway_image_info module with direct API call to the public Marketplace API
  3. Migrates to modern project parameter - Updates scaleway_compute calls to use project instead of deprecated organization parameter
  4. Supports environment variable - Allows setting SCW_DEFAULT_ORGANIZATION_ID to skip the prompt

Technical Details

  • Scaleway's default Project ID equals the Organization ID
  • The Marketplace API (api-marketplace.scaleway.com) is public and doesn't require authentication
  • The project parameter was added to scaleway_compute in community.general 4.3.0
  • This approach matches how other cloud providers (like DigitalOcean) handle similar situations in Algo

Changes

Modified Files

  • roles/cloud-scaleway/tasks/prompts.yml - Added organization/project ID prompt
  • roles/cloud-scaleway/tasks/main.yml - Removed broken modules, added Marketplace API lookup, updated to use project parameter
  • tests/unit/test_scaleway_fix.py - Added 4 new unit tests

Testing

Unit Tests:

  • ✅ All 91 unit tests pass (including 4 new Scaleway-specific tests)
  • ✅ Tests validate: no broken modules, modern project parameter usage, Marketplace API integration, organization ID collection

Linting:

  • ✅ ansible-lint - Passed
  • ✅ yamllint - Passed
  • ✅ ruff (Python) - Passed
  • ✅ shellcheck - Passed
  • ✅ Ansible playbook syntax check - Passed

Limitations

⚠️ Not tested with actual Scaleway deployment - This fix has been validated through unit tests and code review, but hasn't been tested with real Scaleway API credentials. Manual testing would be valuable.

How to Test

To test this fix:

  1. Have a Scaleway account with an API token
  2. Find your Organization ID at https://console.scaleway.com/organization/settings
  3. Run ./algo and select Scaleway as the provider
  4. Enter your Organization ID when prompted
  5. Verify the deployment completes successfully

Alternatively, set environment variables:

export SCW_TOKEN="your-secret-key"
export SCW_DEFAULT_ORGANIZATION_ID="your-org-id"
./algo

Related Issues

…ixes #14846)

The scaleway_organization_info Ansible module is broken and returns empty
data due to using a deprecated API endpoint (upstream issue
ansible-collections/community.general#3782). This causes deployments to
fail with "'NoneType' object has no attribute 'get'" error.

Changes:
- Replace broken scaleway_organization_info module with user prompt for
  Organization/Project ID
- Use Scaleway Marketplace API for image lookup instead of broken
  scaleway_image_info module
- Migrate from deprecated 'organization' parameter to modern 'project'
  parameter in scaleway_compute calls
- Add support for SCW_DEFAULT_ORGANIZATION_ID environment variable
- Provide clear instructions for finding Organization ID in console

Technical details:
- Scaleway's default project ID equals the organization ID
- Marketplace API is public and doesn't require authentication
- The 'project' parameter was added in community.general 4.3.0

Testing:
- Added 4 new unit tests validating the fix
- All 91 unit tests pass
- Passes ansible-lint, yamllint, ruff, and shellcheck

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@dguido dguido requested a review from jackivanov as a code owner October 27, 2025 20:59
@claude
Copy link
Contributor

claude bot commented Oct 27, 2025

Claude finished @dguido's task —— View job


PR Review: Scaleway Deployment Fix

Review Progress

  • Analyze changed files for code quality and bugs
  • Review security implications of API calls and input handling
  • Evaluate test coverage and effectiveness
  • Check for edge cases and error handling
  • Assess performance considerations
  • Provide comprehensive feedback

🔍 Overall Assessment

This is a well-crafted fix that addresses a legitimate issue with broken upstream Ansible modules. The solution is pragmatic and follows established patterns in the Algo codebase.

✅ Strengths

Code Quality

  • Clean migration approach: Removing broken modules (scaleway_organization_info, scaleway_image_info) and replacing with working alternatives
  • Modern parameter usage: Correctly migrates from deprecated organization to project parameter
  • Consistent patterns: Follows existing cloud provider patterns (similar to DigitalOcean's approach)
  • Good separation of concerns: Prompts are properly separated in prompts.yml

Test Coverage

  • Comprehensive validation: 4 focused unit tests covering all key changes
  • Regression prevention: Tests specifically check that broken modules aren't reintroduced
  • Good test structure: Clear, descriptive test names and assertions

⚠️ Security Considerations

API Usage - SECURE ✓

  • Public API endpoint: api-marketplace.scaleway.com is public and doesn't require authentication
  • No credential exposure: API calls don't include sensitive tokens
  • HTTPS only: All API calls use HTTPS

Input Handling - MINOR CONCERNS

  • User input validation: Organization ID input is not validated (could be improved)
  • No sanitization: User input goes directly to API calls (low risk since it's UUID format)

Credential Management - SECURE ✓

  • Proper no_log: Sensitive data is properly protected with no_log: true
  • Environment variable support: Allows secure credential handling via SCW_TOKEN

🐛 Potential Issues

Error Handling

roles/cloud-scaleway/tasks/main.yml:15-20

scaleway_image_id: >-
  {{ (marketplace_images.json.images |
      selectattr('name', 'match', '.*Ubuntu.*22\\.04.*Jammy.*') |
      first).versions[0].local_images |
      selectattr('zone', 'equalto', algo_region) |
      map(attribute='id') | first }}

Issue: This complex Jinja2 expression has multiple failure points:

  1. Empty results: first filter will fail if no Ubuntu 22.04 images match
  2. Missing region: selectattr('zone', 'equalto', algo_region) could return empty if region not available
  3. No error context: Failures would be cryptic Jinja2 errors

Recommendation: Add error handling and validation:

- name: Validate marketplace response
  fail:
    msg: "No images returned from Scaleway Marketplace API"
  when: marketplace_images.json.images | length == 0

- name: Find Ubuntu 22.04 Jammy image
  set_fact:
    ubuntu_images: "{{ marketplace_images.json.images | selectattr('name', 'match', '.*Ubuntu.*22\\.04.*Jammy.*') | list }}"

- name: Validate Ubuntu image found
  fail:
    msg: "No Ubuntu 22.04 Jammy images found in marketplace"
  when: ubuntu_images | length == 0

Input Validation

roles/cloud-scaleway/tasks/prompts.yml:33-36

Issue: Organization ID format is not validated. While UUID format is expected, malformed input could cause API failures.

Recommendation: Add basic UUID format validation:

- name: Validate organization ID format
  fail:
    msg: "Organization ID must be a valid UUID format"
  when: not (algo_scaleway_org_id | regex_search('^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$'))

🚀 Performance Considerations

API Efficiency - GOOD ✓

  • Single API call: Fetches all images in one request
  • Client-side filtering: Uses Jinja2 filters instead of multiple API calls
  • Reasonable payload: Marketplace API returns manageable data size

Caching Opportunities

Minor optimization: The marketplace API call could be cached since image IDs don't change frequently, but this is not critical for Algo's use case.

📋 Code Quality Issues

Magic Strings

roles/cloud-scaleway/tasks/main.yml:17

selectattr('name', 'match', '.*Ubuntu.*22\\.04.*Jammy.*')

Issue: Hardcoded regex pattern should be configurable or at least extracted to a variable.

Recommendation:

vars:
  ubuntu_image_pattern: ".*Ubuntu.*22\\.04.*Jammy.*"

Duplicate Code

roles/cloud-scaleway/tasks/main.yml:22-37 and 50-64

Issue: The scaleway_compute task is duplicated (create vs start) with identical parameters.

Recommendation: Consider using state: running in a single task, or extract common parameters to variables.

📝 Documentation

User Experience - EXCELLENT ✓

  • Clear prompts: Well-written user guidance with specific URLs
  • Environment variable docs: Good support for automation via env vars
  • Step-by-step instructions: Clear instructions for finding Organization ID

🧪 Testing Recommendations

Current Tests - GOOD ✓

The unit tests are well-structured and cover the key functionality. However, consider adding:

  1. API response validation tests: Mock marketplace API responses and test edge cases
  2. Jinja2 template tests: Validate the complex image filtering logic
  3. Integration test guidance: Since real API testing isn't performed

Suggested Additional Tests

def test_image_filtering_edge_cases():
    """Test that image filtering handles edge cases gracefully"""
    # Test empty API response
    # Test missing Ubuntu images
    # Test region availability

🔧 Minor Improvements

  1. Add retry logic to marketplace API call (network failures)
  2. Add timeout to API requests
  3. Consider logging the selected image ID for debugging
  4. Variable naming: algo_scaleway_org_id could be scaleway_project_id for clarity

✅ Final Recommendation

APPROVE with minor suggestions

This PR successfully solves a real deployment issue with a clean, well-tested approach. The security posture is good, and the code follows Algo's established patterns. The suggested improvements are minor and could be addressed in future PRs if desired.

The approach of prompting users for Organization ID is much more reliable than depending on broken upstream modules, and the Marketplace API usage is appropriate for this use case.

assert 'organization: "{{' not in content, "Still using deprecated 'organization' parameter"

# Should use Marketplace API for image lookup
assert "api-marketplace.scaleway.com" in content, "Not using Scaleway Marketplace API for image lookup"

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
api-marketplace.scaleway.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 11 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

), "Missing support for SCW_DEFAULT_ORGANIZATION_ID environment variable"

# Should mention console.scaleway.com for finding the ID
assert "console.scaleway.com" in content, "Missing instructions on where to find Organization ID"

Check failure

Code scanning / CodeQL

Incomplete URL substring sanitization High test

The string
console.scaleway.com
may be at an arbitrary position in the sanitized URL.

Copilot Autofix

AI 11 days ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

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.

[ERROR]: Task failed: Module failed: 'NoneType' object has no attribute 'get' Origin: /roles/cloud-scaleway/tasks/main.yml:6:7

2 participants