getmoto / moto

A library that allows you to easily mock out tests based on AWS infrastructure.
http://docs.getmoto.org/en/latest/
Apache License 2.0
7.6k stars 2.03k forks source link

modify_option_group does not receive OptionsToInclude value #8144

Closed aucheun-tu closed 3 days ago

aucheun-tu commented 5 days ago

When I attempt to use moto to mock the modify_option_group call for the RDS client. It seems the OptionsToInclude value does not properly get passed into the call and moto ends up setting the value of options_to_include to an empty list, []. That leads to the following error: An error occurred (InvalidParameterValue) when calling the ModifyOptionGroup operation: At least one option must be added, modified, or removed..

Here is an example of a pytest I setup to demonstrate this:

    @mock_aws
    def test_example(self):
        client = boto3.client("rds", region_name="us-east-1")

        client.create_option_group(
            OptionGroupName="testoptiongroupname",
            EngineName="sqlserver-ee",
            MajorEngineVersion="11.00",
            OptionGroupDescription="testdescription",
        )

        client.modify_option_group(
            OptionGroupName="testoptiongroupname",
            OptionsToInclude=[{"OptionName": "TDE"}],
            ApplyImmediately=True,
        )

I also modified the modify_option_group function in the moto/rds/models.py file with a print statement before running the example test:

    def modify_option_group(
        self,
        option_group_name: str,
        options_to_include: Optional[List[Dict[str, Any]]] = None,
        options_to_remove: Optional[List[Dict[str, Any]]] = None,
    ) -> "OptionGroup":
        if option_group_name not in self.option_groups:
            raise OptionGroupNotFoundFaultError(option_group_name)
        # PRINT STATEMENT TO DEBUG VALUE OF options_to_include
        print(f"the value of options_to_include is: {options_to_include}")
        if not options_to_include and not options_to_remove:
            raise RDSClientError(
                "InvalidParameterValue",
                "At least one option must be added, modified, or removed.",
            )

Running the test above along with the additional print statement added to moto results in the following error and print:

E           botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the ModifyOptionGroup operation: At least one option must be added, modified, or removed.

--------------------------------------- Captured stdout call ---------------------------------------
the value of options_to_include is: []

This seems to indicate despite the fact that I pass in [{"OptionName": "TDE"}] to OptionsToInclude, moto is not receiving the value properly and setting it to [].

Looking at the test_modify_option_group test in the tests/test_rds/test_rds.py file, there is a comment that seems to indicate the developer of the tests ran into this same issue. Ref: https://github.com/getmoto/moto/blob/master/tests/test_rds/test_rds.py#L1480 Tagging @bblommers in case you know anything as it looks like this is your comment. Comment from the test:

    # TODO: create option and validate before deleting.
    # if Someone can tell me how the hell to use this function
    # to add options to an option_group, I can finish coding this.

Please advise on if I am mocking modify_option_group properly of if this is a bug in moto. Thanks!

bblommers commented 5 days ago

Hi @aucheun-tu, thank you for raising this, and welcome to Moto!

That code was originally committed 10 years ago by someone else (see https://github.com/getmoto/moto/commit/6fad81aabfbdf65f3d463e9ca568ae95f94698d8) - I only refactored it last.

Regardless - it is indeed a bug. I've opened a PR that improves support for this. The test_example function that you provide should work with this change.

bblommers commented 4 days ago

@aucheun-tu moto >= 5.0.16.dev5 should fix the problem - are you able to upgrade and verify this?

aucheun-tu commented 4 days ago

@bblommers I updated to moto >= 5.0.16.dev5 and the issue was resolved. thanks for the quick response!

bblommers commented 3 days ago

Great, happy to hear that @aucheun-tu.

I'll close this issue, but let us know if you run into any other issues!