Feature: configurable plugins

[jmartin-tech]

Fix #589
Fix #595

Buckle up lots of adjustments to review. Many of the generator/probe/detector/buff/harness plugins are refactored in some way or another.

The goal of this change is to offer configuration of instance variables on plugins on a per "type" basis.

Summary of changes:

• all plugins implementing Configurable and accept a named config_root parameter that defaults to the global _config
• shift all configurable class variables into DEFAULT_PARAMS to avoid leaking instance specific values
• inject all default params as instance attributes
• provide exmaple for _supported_params in class definitions
• propagate keys in the plugin yaml dictionalry as attributed on plugin instantiation
• mechanism based on ENV_VAR class constant to obtain api_key during instantiation
• plugin provides defining a class level _supported_params only apply supported params from configuration files, a warning is logged for unknown items
• refactor inline configuration yaml refactored for readability
• rasa.RasaRestGenerator is implements an extension of rest.RestGenerator with embedded defaults and updated module config docs to increase code reuse
• garak.generators.load_generator() and other generator loading code is removed in favor of _plugin.load_plugin() accounting for DEFAULT_CLASS
• remove plugin configuration method in favor of Configurable

Configuration files apply in a hierarchal fashion and allow configuration of parameters not exposed in the class initializer.

Final values for configured attributes as instance variables are applied in with precedence to programatic input with a nuance that a constructor default value can be overwritten by a configuration file value if the provided value is the same as the default.

value from named parameter in constructor 
  else: 
    value from config passed to constructor
    else:
      default provided or inherited in class

The changes introduce breaking changes to the expected structure of configuration wether provided as yaml or json.
The primary breaking change is the expectation that configuration of a class will be nested inside configuration for all classes of a module holding more specific resolutions as the maintained values. This format change is currently left in a partially compatible state to provide time for consumers to update configuration formats.
Here is an example of a how configuration of the rest.Generator class is expected to change.

        {"rest.RestGenerator":
            {
                "name": "example service",
                "uri": "https://example.ai/llm",
                "method": "post",
                "headers":{
                    "X-Authorization": "$KEY",
                },
                "req_template_json_object":{
                    "text":"$INPUT"
                },
                "response_json": true,
                "response_json_field": "text"
            }
        }

Becomes:

      {
          "rest": {
              "RestGenerator": {
                  "name": "example service",
                  "uri": "https://example.ai/llm",
                  "method": "post",
                  "headers": {
                      "X-Authorization": "$KEY",
                  },
                  "req_template_json_object": {
                      "text": "$INPUT"
                  },
                  "response_json": true,
                  "response_json_field": "text"
              }
          }
      }

If provided the previous format a warning is logged in garak.log about used of deprecated formatting.

[leondz]

Looking good. Few questions, many around precendence/parent class.

[leondz]

OK, I can definitely see that `uri` on its own is always ambiguous, we can shift that other use to doc_uri. Leaving it as a class attribute (immutable) seems like it might be enough signal re: not updating it?

…

On Fri, May 31, 2024, 18:03 Jeffrey Martin ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In garak/generators/langchain_serve.py <#711 (comment)>: > + if self.uri is None: + self.uri = os.getenv("LANGCHAIN_SERVE_URI") + if not self._validate_uri(self.uri): I see what you are referring to, at the class level uri is being reserved in some plugins. I land the other direction, I would say the uri attribute should not be reserved for "references" and it would be more clear to define specific documentation in an attribute with a lower probability for collision and even possibly as a list, something like references, public_sources, of maybe public_references. Maybe even use a constant like REFERENCES to denote that the value is not expected to ever be manipulated in by an instance. — Reply to this email directly, view it on GitHub <#711 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA5YTS6CDCRJFJU6J4ZUWLZFCNL5AVCNFSM6AAAAABIR4IWXKVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAOJRGE4TONZQGQ> . You are receiving this because your review was requested.Message ID: ***@***.***>

[erickgalinkin]

Looks really good. Left a handful of comments throughout.

[jmartin-tech]

Thanks for all the feedback, I will work thru it all and post updated code soon.

[erickgalinkin]

Looks good to me. Definitely some work to be done refactoring TAP and AutoDAN but that's a problem for me to get into next week.

EDIT: Looks like some tests are failing, so clearly I've missed something.

Failing tests

[erickgalinkin]

LGTM.