Add oci provider

[sshmulev]

No description provided.

[sourcery-ai]

Hey - I've found 5 issues, and left some high level feedback:

• In get_instances_oci, values['source_details'][0]['source_id'] and values.get('create_vnic_details', [{}])[0] assume list-of-objects, but OCIConfigBuilder currently defines source_details and create_vnic_details as plain dicts, so either the builder or the controller should be aligned to use the same structure to avoid runtime errors.
• The ensure_rpm_usable_before_tests autouse fixture now requires the instance_data fixture; this can break modules that don't provide instance_data, so consider making instance_data optional (e.g., via request or a default) or scoping this fixture only to modules where instance_data is guaranteed to exist.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `get_instances_oci`, `values['source_details'][0]['source_id']` and `values.get('create_vnic_details', [{}])[0]` assume list-of-objects, but `OCIConfigBuilder` currently defines `source_details` and `create_vnic_details` as plain dicts, so either the builder or the controller should be aligned to use the same structure to avoid runtime errors.
- The `ensure_rpm_usable_before_tests` autouse fixture now requires the `instance_data` fixture; this can break modules that don't provide `instance_data`, so consider making `instance_data` optional (e.g., via `request` or a default) or scoping this fixture only to modules where `instance_data` is guaranteed to exist.

## Individual Comments

### Comment 1
<location path="cloud/opentofu/opentofu_controller.py" line_range="188" />
<code_context>
+                'private_ip': values.get('private_ip', ''),
+                'availability_domain': values['availability_domain'],
+                'shape': values['shape'],
+                'image': values['source_details'][0]['source_id'],
+                'username': self.tf_configurator.get_oci_username_by_instance_name(resource['name']),
+            }
</code_context>
<issue_to_address>
**issue (bug_risk):** Align `source_details` structure between OCI config builder and controller.

`OCIConfigBuilder.__new_oci_instance` emits `source_details` as a dict:
```python
'source_details': {
    'source_type': 'image',
    'source_id': instance['image_id'],
}
```
But here it’s read as a list (`values['source_details'][0]['source_id']`). This inconsistency will either raise at runtime or mis-parse the state. Please align both sides: either make the builder emit a list of dicts, or update the controller to treat `source_details` as a dict (e.g. `values['source_details']['source_id']`).
</issue_to_address>

### Comment 2
<location path="cloud/opentofu/oci_config_builder.py" line_range="92-100" />
<code_context>
+        if not self.__get_tf_resource_name_by_region('oci_core_default_route_table', region):
+            declared_default_rt_id = f'oci_core_vcn.{vcn_name}.default_route_table_id'
+            declared_ig_id = f'oci_core_internet_gateway.{ig_name}.id'
+            self.resources_tf['resource']['oci_core_default_route_table'][rt_name] = {
+                'provider': f'oci.{region}',
+                'manage_default_resource_id': f'${{{declared_default_rt_id}}}',
+                'route_rules': {
+                    'network_entity_id': f'${{{declared_ig_id}}}',
+                    'destination': '0.0.0.0/0',
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider using a list for `route_rules` to match OCI Terraform schema expectations.

`route_rules` is defined as a single dict, but the Terraform provider expects a list of route rule blocks. This mismatch may cause the generated config to be rejected or misinterpreted. Please emit `route_rules` as a list of dicts (even for a single rule), e.g. `"route_rules": [{"network_entity_id": ..., "destination": ...}]`, to align with the OCI schema.

```suggestion
            self.resources_tf['resource']['oci_core_default_route_table'][rt_name] = {
                'provider': f'oci.{region}',
                'manage_default_resource_id': f'${{{declared_default_rt_id}}}',
                'route_rules': [{
                    'network_entity_id': f'${{{declared_ig_id}}}',
                    'destination': '0.0.0.0/0',
                    'destination_type': 'CIDR_BLOCK',
                }],
            }
```
</issue_to_address>

### Comment 3
<location path="cloud/opentofu/oci_config_builder.py" line_range="148-150" />
<code_context>
+                'source_type': 'image',
+                'source_id': instance['image_id'],
+            },
+            'create_vnic_details': {
+                'subnet_id': f'${{{declared_subnet_id}}}',
+                'assign_public_ip': True,
+            },
+            'metadata': {
</code_context>
<issue_to_address>
**issue (bug_risk):** Make `create_vnic_details` structure consistent with how the controller reads it.

Here `create_vnic_details` is a dict, but `get_instances_oci` treats it as a list and accesses index `[0]`. Please align these (either emit a list here or update the reader to expect a dict) to avoid runtime errors when parsing the Terraform state.
</issue_to_address>

### Comment 4
<location path="cloud/opentofu/opentofu_configurator.py" line_range="97-102" />
<code_context>

         raise Exception(f'ERROR: No instance with name "{ami_name}" was found')
+
+    def get_oci_username_by_instance_name(self, tf_resource_name):
+        for instance in self.resources_dict['instances']:
+            sanitized = instance['name'].replace('.', '-')
+            if sanitized in tf_resource_name:
+                return instance['username']
+        raise Exception(f'ERROR: No OCI instance matching tf resource name "{tf_resource_name}" was found')
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Tighten matching logic between resource name and OCI instance to avoid ambiguous matches.

Using `if sanitized in tf_resource_name` allows substring matches (e.g. `web` matching `web-prod`), which can select the wrong username. Please use a stricter condition (e.g. exact match on the intended suffix or a delimiter-based match) to ensure unambiguous mapping.
</issue_to_address>

### Comment 5
<location path="test_suite/cloud/test_oci.py" line_range="35-36" />
<code_context>
+        """
+        Verify the deployed instance metadata matches what CIV expects.
+        """
+        assert instance_data_oci_web.get('region') == instance_data.get('availability_domain', '').split(':')[0].lower() \
+               or instance_data_oci_web.get('canonicalRegionName'), \
+            'Instance region from metadata does not match expected region'
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Region identity assertion can pass even when regions mismatch, making the test misleading.

Because the assertion is `region == expected or canonicalRegionName`, any non-empty `canonicalRegionName` will cause the test to pass even when `region` and `expected` differ, so mismatches won’t be caught.

Instead, first assert that at least one region field is present, then explicitly compare the actual region to the expected value, e.g.:

```python
region = instance_data_oci_web.get('region')
canonical_region = instance_data_oci_web.get('canonicalRegionName')
expected_region = instance_data.get('availability_domain', '').split(':')[0].lower()

assert region or canonical_region, "Region metadata not available from IMDS"
if region:
    assert region == expected_region, (
        f"Instance region from metadata ({region}) does not match expected ({expected_region})"
    )
```

This ensures the test fails on real mismatches instead of being short-circuited by `canonicalRegionName` being present.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[sourcery-ai]

issue (bug_risk): Align source_details structure between OCI config builder and controller.

OCIConfigBuilder.__new_oci_instance emits source_details as a dict:

'source_details': {
    'source_type': 'image',
    'source_id': instance['image_id'],
}

But here it’s read as a list (values['source_details'][0]['source_id']). This inconsistency will either raise at runtime or mis-parse the state. Please align both sides: either make the builder emit a list of dicts, or update the controller to treat source_details as a dict (e.g. values['source_details']['source_id']).

[sourcery-ai]

suggestion (bug_risk): Consider using a list for route_rules to match OCI Terraform schema expectations.

route_rules is defined as a single dict, but the Terraform provider expects a list of route rule blocks. This mismatch may cause the generated config to be rejected or misinterpreted. Please emit route_rules as a list of dicts (even for a single rule), e.g. "route_rules": [{"network_entity_id": ..., "destination": ...}], to align with the OCI schema.

Suggested change

	self.resources_tf['resource']['oci_core_default_route_table'][rt_name] = {
	'provider': f'oci.{region}',
	'manage_default_resource_id': f'${{{declared_default_rt_id}}}',
	'route_rules': {
	'network_entity_id': f'${{{declared_ig_id}}}',
	'destination': '0.0.0.0/0',
	'destination_type': 'CIDR_BLOCK',
	},
	}
	self.resources_tf['resource']['oci_core_default_route_table'][rt_name] = {
	'provider': f'oci.{region}',
	'manage_default_resource_id': f'${{{declared_default_rt_id}}}',
	'route_rules': [{
	'network_entity_id': f'${{{declared_ig_id}}}',
	'destination': '0.0.0.0/0',
	'destination_type': 'CIDR_BLOCK',
	}],
	}

[sourcery-ai]

suggestion (bug_risk): Tighten matching logic between resource name and OCI instance to avoid ambiguous matches.

Using if sanitized in tf_resource_name allows substring matches (e.g. web matching web-prod), which can select the wrong username. Please use a stricter condition (e.g. exact match on the intended suffix or a delimiter-based match) to ensure unambiguous mapping.

[sourcery-ai]

issue (bug_risk): Region identity assertion can pass even when regions mismatch, making the test misleading.

Because the assertion is region == expected or canonicalRegionName, any non-empty canonicalRegionName will cause the test to pass even when region and expected differ, so mismatches won’t be caught.

Instead, first assert that at least one region field is present, then explicitly compare the actual region to the expected value, e.g.:

region = instance_data_oci_web.get('region')
canonical_region = instance_data_oci_web.get('canonicalRegionName')
expected_region = instance_data.get('availability_domain', '').split(':')[0].lower()

assert region or canonical_region, "Region metadata not available from IMDS"
if region:
    assert region == expected_region, (
        f"Instance region from metadata ({region}) does not match expected ({expected_region})"
    )

This ensures the test fails on real mismatches instead of being short-circuited by canonicalRegionName being present.