feat(extension): add support for custom canister types

[smallstepman]

Description

The PR extends functionalities to adapt to the new feature of custom canister types. The changes accommodate:

• parsing of canister_types in the extension manifest file
• transformation of .canisters.<CAN> in dfx.json based on extension's canister_types declaration

Note:

• depends on chore: move extensions to dfx-core #3221
• unusable outside of https://github.com/dfinity/dfx-extensions until this lands: feat(extensions): install from external registries  #2968

Closes https://dfinity.atlassian.net/browse/SDK-1073

How Has This Been Tested?

• unit
• e2e
• manually

Checklist:

• The title of this PR complies with Conventional Commits.
• I have edited the CHANGELOG accordingly.
• I have made corresponding changes to the documentation.

[lastmjs]

Question on this, if someone is using a custom type like azle or kybra, will they still be able to provide regular dfx.json properties? For example, I'd like to allow Azle and Kybra developers to override the simple azle and kybra canister configurations with the underlying dfx.json properties if they want to. Let's say they want to turn off gzip, they should be able to. Or if they want to turn on optimization or change it, they should be able to.

[smallstepman]

Thanks for bringing that up; it definitely should work just as you've described it, I'll make a test case for it

[smallstepman]

I am not sure what kind of error to use over here

[ericswanson-dfinity]

Since from_slice will now have new failure modes, we'll have to change the return type.

We can reduce the scope of those failure modes, though.

How about this?

1. Instantiate the ExtensionManager in the caller

• make EnvironmentImpl::new() create the ExtensionManager.
• rename EnvironmentImpl::new_extension_manager()` and

2. Pass a trait rather than the whole ExtensionManager

• make trait TransformConfiguration
  • single method: transform(&mut json) -> Result<(), TransformConfigurationError>
  • either pass Option<TransformConfiguration> to these methods, or for tests pass an object that is noop for this method

3. Change these methods' error type from StructuredFileError to ReadConfigurationError (possibilities: StructuredFileError, TransformConfigurationError)

This way the error return type of from_slice and friends will reflect the errors that can occur, but won't also have to include errors related to instantiating the extension manager.

[smallstepman]

@ericswanson-dfinity I just realized this is buggy... I'm working on a fix

[smallstepman]

After all, I'm not sure what should be the desired behavior...
I extended unit tests with:

    #[test]
    fn test_op_replace_basic() {
        test_op!(
            custom_canister_template = r#"
        {
            "main": { "replace": { "input": "{{canister_name}}", "search": "frontend_(.*)", "output": "thecanister/$1/main.ts" } }
        }
        "#,
            dfx_json_canister_values = r#"
        {
            "canister_name": "frontend_xyz"
        }
        "#,
            expected = r#"
        {
            "main": "thecanister/xyz/main.ts"
        }
        "#
        );
    }

    #[test]
    fn test_op_replace_nested() {
        test_op!(
            custom_canister_template = r#"
        {
            "main": { "replace": { "input": "{{main}}", "search": "(.*)", "output": "thecanister/$1" } }
        }
        "#,
            dfx_json_canister_values = r#"
        {
            "main": "src/main.ts"
        }
        "#,
            expected = r#"
        {
            "main": "thecanister/src/main.ts"
        }
        "#
        );
    }

the _basic will pass, but what should be the expected outcome for _nested? This is related to #3222 (comment)

[ericswanson-dfinity]

I don't understand how this is related to the comment.

From reading the comment I think the expectation is that

{
    "type": "kybra",
    "gzip": "true"
}

should transform according to the rules of the kybra extension, but also keep the "gzip" field.

Also I think these tests would benefit from some prose descriptions of what's being tested, what's the expected behavior, and what are the nuances. As a reader I don't want to have to parse a regex in my head in order to figure out what the test is checking.

[smallstepman]

apologies, I mixed too much info in that example; let me untangle. Going from the simplest to the most complex:

1. Append user-defined field: This is when a user adds a field, like gzip: true, to their dfx.json file, despite gzip not being defined in the canister template. This currently works as expected.

   #[test]
   fn test_op_replacement_1() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "something.py"
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "gzip": true
       }
       "#,
           expected = r#"
       {
           "gzip": true,
           "main": "something.py"
       }
       "#
       );
   }

2. Override the field: This is when a field such as gzip: false is defined in the canister template but the user modifies its value in their dfx.json, like gzip: true. This modification also works as expected.

   #[test]
   fn test_op_replacement_2() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "something.py",
           "gzip": false
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "gzip": true
       }
       "#,
           expected = r#"
       {
           "gzip": true,
           "main": "something.py"
       }
       "#
       );
   }

3. Nested field override, while using the field as a template for another field: This case happens when a field defined in the canister template is overridden by the user in their dfx.json, and the initial value of this field is employed as a placeholder in another field in the template. This function is also performing as expected for now.

   #[test]
   fn test_op_replacement_3() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "{{gzip}}.py",
           "gzip": false
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "gzip": true
       }
       "#,
           expected = r#"
       {
           "gzip": true,
           "main": "true.py"
       }
       "#
       );
   }

4. Nested field override, using the field's original value as a placeholder for the same field: This is the scenario when a field's value in the canister template is both overridden by the user in their dfx.json and used as a placeholder within the same field. For instance, if main in the template is main: "path/to/{{main}}", and the user has main: "something.py" in dfx.json, the expected result should be main: "path/to/something.py". However, the present tests for this case are failing.

   #[test]
   fn test_op_replacement_4() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "path/to/{{main}}"
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "main": "something.py"
       }
       "#,
           expected = r#"
       {
           "main": "path/to/something.py"
       }
       "#
       );
   }

   ---- extension::manifest::custom_canister_type::tests::test_op_replacement_4 stdout ----
   thread 'extension::manifest::custom_canister_type::tests::test_op_replacement_4' panicked at 'assertion failed: `(left == right)`
     left: `{"main": String("something.py")}`,
    right: `{"main": String("path/to/something.py")}`', src/dfx-core/src/extension/manifest/custom_canister_type.rs:307:9
   note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

5. Nested field override with a replacement operation: This case involves a user-defined replacement operation in the dfx.json, where the value for the "main" field is used as a placeholder. This value gets replaced through a regex operation defined in the canister template. For example, if main in the template is "main": { "replace": { "input": "{{main}}", "search": ".*/(.*).ts", "output": "thecanister/$1.exe" } }, and the user has main: "src/main.ts" in dfx.json, the expected result should be main: "thecanister/main.exe". Currently, the tests for this scenario are also failing.

   #[test]
   fn test_op_replacement_5() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": { "replace": { "input": "{{main}}", "search": ".*/(.*).ts", "output": "thecanister/$1.exe" } }
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "main": "src/main.ts"
       }
       "#,
           expected = r#"
       {
           "main": "thecanister/main.exe"
       }
       "#
       );
   }

   ---- extension::manifest::custom_canister_type::tests::test_op_replacement_5 stdout ----
   thread 'extension::manifest::custom_canister_type::tests::test_op_replacement_5' panicked at 'assertion failed: `(left == right)`
     left: `{"main": String("src/main.ts")}`,
    right: `{"main": String("thecanister/main.exe")}`', src/dfx- core/src/extension/manifest/custom_canister_type.rs:328:9

let me know if you agree all of these should pass

[ericswanson-dfinity]

Thanks, these are great explanations. I think it would be helpful to include these explanations as comments in the tests themselves. What do you think?

As for the scenarios:

1. agree
2. agree
3. agree, though I wonder: for a field like this where the type template is using one field as input to generate another field, wouldn't it be common for the template to include an op to remove the input field? Also, "gzip" might be a counterintuitive name for the field that demonstrates this behavior.
4. I would expect this test to pass as written
5. I would expect this test to pass as written

[smallstepman]

Thanks, it's good that we established this baseline. Yes, adding these comments to the code seems like a good idea. I agree that using gzip in example #3 is counterintuitive.

Adding remove: true adds another layer of complexity, which I think should be discussed separately.

I laid out these tests like one in the previous post; hope that's easy enough to follow.

1. Remove the field, if it was defined in dfx.json

   #[test]
   fn test_op_replace_and_delete_1() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "something.py",
           "gzip": { "remove": true }
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "gzip": true
       }
       "#,
           expected = r#"
       {
           "main": "something.py"
       }
       "#
       );
   }

2. Disallow overwriting the field?

    #[test]
    fn test_op_replace_and_delete_2() {
        test_op!(
            custom_canister_template = r#"
        {
            "main": "something.py",
            "gzip": false,
            "gzip": { "remove": true }
        }
        "#,
            dfx_json_canister_values = r#"
        {
            "gzip": true
        }
        "#,
            expected = r#"
        {
            "gzip": false,
            "main": "something.py"
        }
        "#
        );
    }

3. unclear what the expected behavior should be here

   #[test]
   fn test_op_replace_and_delete_3() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "{{gzip}}.py",
           "gzip": false,
           "gzip": { "remove": true }
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "typ": true
       }
       "#,
           expected = r#"
       {
           "gzip": false,
           "main": "true.py"
       }
       "#
       );
   }

4. unclear what the expected behavior should be here

   #[test]
   fn test_op_replace_and_delete_4() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": "path/to/{{main}}",
           "main": { "remove": true }
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "main": "something.py"
       }
       "#,
           expected = r#"
       {
           "main": "path/to/something.py"
       }
       "#
       );
   }

5. unclear what the expected behavior should be here

   #[test]
   fn test_op_replace_and_delete_5() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": { "replace": { "input": "{{main}}", "search": ".*/(.*).ts", "output": "thecanister/$1.exe" } },
           "main": { "remove": true }
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "main": "src/main.ts"
       }
       "#,
           expected = r#"
       {
           "main": "thecanister/main.exe"
       }
       "#
       );
   }

6. unclear what the expected behavior should be here

   #[test]
   fn test_op_replace_and_delete_6() {
       test_op!(
           custom_canister_template = r#"
       {
           "main": { "replace": { "input": "{{canister_name}}", "search": "frontend_(.*)", "output": "thecanister/$1/main.ts" } }
       }
       "#,
           dfx_json_canister_values = r#"
       {
           "canister_name": "frontend_xyz"
       }
       "#,
           expected = r#"
       {
           "main": "thecanister/xyz/main.ts"
       }
       "#
       );
   }

[ericswanson-dfinity]

Let's start with documentation in docs/concepts describing how to define an custom canister type and how to use one in your project. This will help with the review for questions like "what is "type": "<x>:<y>"

[ericswanson-dfinity]

Replaced by #3641