RFC: New registry provider extensibility model

[bwateratmsft]

Docker Registry Provider Extensibility Model

Motivation

Several concerns around registry providers have motivated us to explore a better extensibility model for supplying additional providers. These include:

1. Scalability--we can't scale to implement and maintain a multitude of different registry providers
2. Openness--we don't want to be the arbiters of which registries get implemented and shown

In order to alleviate these concerns it is necessary to establish a better extensibility model.

Overview

Adding a registry provider would involve creating an extension, which can communicate with the Docker extension to provide a view into a given registry. Optionally, the extension could also implement context and palette commands. With the correct context values on nodes, the existing context and palette commands will also work. These commands include:

• vscode-docker.registries.copyImageDigest
• vscode-docker.registries.deleteImage
• vscode-docker.registries.disconnectRegistry
• vscode-docker.registries.logInToDockerCli
• vscode-docker.registries.logOutOfDockerCli
• vscode-docker.registries.pullImage
• vscode-docker.registries.pullRepository
• vscode-docker.registries.reconnectRegistry
• vscode-docker.images.push (indirectly, this depends on DockerRegistry.baseImagePath)

// TODO: this feels out of place in the Overview section

// TODO: List the right context values in source

A Node package (vscode-docker-registries) contains the interfaces necessary to properly implement the provider, and also includes a generic V2 provider, since most providers would be little more than a slim inheriting implementation on top of that.

Implementing a registry provider

In order to connect to a given registry, the provider author must write the necessary code to authenticate and query their offering, and implement the necessary provider interfaces (alluded to above and described in detail below).

There are two ways to do this:

1. Strict registry structure

An extension implements three primary methods (and some additional supporting methods):

1. Get the list of registries (or just one if it's a public registry)
2. For each registry, get the list of repositories
3. For each repository, get the list of tags

Implementing a registry provider this way is very simple, but additional features (like ACR's subscriptions, tasks) are not possible.

2. Show arbitrary tree structure

An extension gives a tree object which will show anything they wish. It is more difficult to implement but offers greater flexibility.

Registering a registry provider

The Docker extension implements an interface (see below) allowing for registry providers to register themselves with the extension. The extensions need to call this registration method every time the Docker extension is activated, and can accomplish this by setting an activation event on the command vscode-docker.registries.providerActivation. In package.json:

{
    ...
    "activationEvents": [
        "onCommand:vscode-docker.registries.providerActivation",
        ...
    ]
    ...
}

Upon activation, the provider extension must call the Docker extension to register. The registerDockerRegistryProvider method returns a Disposable which should be pushed to the extension activation context's subscriptions.

export function activate(ctx: vscode.ExtensionContext): void {
    const provider = new MyDockerRegistryProvider();
    const dockerExtension = vscode.extensions.getExtension<DockerExtension>('ms-azuretools.vscode-docker');
    ctx.subscriptions.push(dockerExtension.registerDockerRegistryProvider(provider));
}

Showing up in the provider pick list

In order to be used, an extension needs to show up in the quick pick list for registry providers. The list will consist of:

1. All providers for which the prerequisite(s) are installed (which would always include Docker Hub and the generic V2 provider), and that have registered themselves at runtime with the Docker extension.
2. A manifest maintained in the Docker Extension repo will keep a list of known provider IDs, names, and dependent extensions. We will accept contributions to this manifest, with reasonable criteria such as:
   • We will spend at least a bit of time up front inspecting their source code for obvious problems
   • Their provider must be open-source
   • Their provider must not be redundant with existing ones
   • It's rather subjective, but their provider must appear like a serious effort that will be maintained in the future; not a passing side project

Optionally, the Docker extension may also establish a tag that can be used by provider extensions to easily filter for them on the marketplace, and a link/command/etc. within the Docker extension to open the marketplace with that filter.

Interfaces

Docker extension

DockerExtension:

/**
 * Interface implemented by the Docker extension.
 * @example const dockerExtension = vscode.extensions.getExtension<DockerExtension>('ms-azuretools.vscode-docker').exports;
 */
export interface DockerExtension {
    /**
     * Registers a registry provider with the Docker extension
     * @param provider The provider to register with the extension
     * @returns A disposable that, when disposed, will un-register the provider
     */
    registerDockerRegistryProvider(provider: BasicDockerRegistryProvider | CustomDockerRegistryProvider): Disposable;
}

Note: "register...Registry" is a bit of a tongue twister, but all of the VSCode API's similar methods use "register*"; consistency is good.

Basic registry provider

RegistryTreeItem:

/**
 * Interface for all nodes that appear in the Docker extension's explorer view for registries.
 * This is mostly-identical to `vscode.TreeItem` but intentionally does not extend it--`RegistryTreeItem` objects
 * created by the provider will not be directly used to build the tree in the VS Code UI, nor will they be passed
 * as context when commands are invoked. Instead, the properties below will be copied into a new object. Any
 * additional properties will not be copied.
 */
export interface RegistryTreeItem {
    /**
     * The label for the node
     */
    readonly label: string;

    /**
     * The context value for the node. In order to use the existing commands for registries in the Docker extension, this must contain // TODO
     */
    readonly contextValue: string;

    /**
     * The ID for the node. Optional. This is not the same as `providerId` / `registryId`.
     */
    readonly id?: string;

    /**
     * The icon for the node
     */
    readonly iconPath?: string | Uri | { light: string | Uri; dark: string | Uri } | ThemeIcon;

    /**
     * The description for the node (rendered less prominently than the label)
     */
    readonly description?: string | boolean;

    /**
     * The tooltip for the node
     */
    readonly tooltip?: string;

    /**
     * The command to run when the node is left-clicked
     */
    readonly command?: Command;
}

DockerRegistryProviderBase:

/**
 * Base interface for registry providers
 */
export interface DockerRegistryProviderBase extends RegistryTreeItem {
    /**
     * The registry provider ID. Must be universally unique and consisting only of alphanumeric characters.
     */
    readonly providerId: string;

    /**
     * Connects a registry. UI prompts should be implemented to gather the necessary information to connect. This method is optional if a provider can determine all the necessary information on its own without prompts.
     * @param token Cancellation token
     */
    connectRegistry?(token: CancellationToken): Promise<DockerRegistry>;

    /**
     * Disconnects a registry
     */
    disconnectRegistry?(registry: DockerRegistry): Promise<void>;
}

BasicDockerRegistryProvider:

/**
 * Interface for a basic registry provider that implements `getRegistries`, to provide a set of registries, repositories, and tags
 */
export interface BasicDockerRegistryProvider extends DockerRegistryProviderBase {
    /**
     * Gets the registries for this provider
     * @param refresh If true, a refresh is being done, and caching should not be used
     * @param token Cancellation token
     */
    getRegistries(refresh: boolean, token: CancellationToken): Promise<DockerRegistry[]>;
}

DockerRegistry:

/**
 * Interface for a Docker registry. A registry contains a set of repositories.
 */
export interface DockerRegistry extends RegistryTreeItem {
    /**
     * The base image path, prepended to the image name when pushing.
     */
    readonly baseImagePath: string;

    /**
     * The registry ID. Must be unique within the registry provider, and consisting only of alphanumeric characters.
     */
    readonly registryId: string;

    /**
     * Gets the repositories that are contained in this registry.
     * @param refresh If true, a refresh is being done, and caching should not be used
     * @param token Cancellation token
     */
    getRepositories(refresh: boolean, token: CancellationToken): Promise<DockerRepository[]>;

    /**
     * Gets the login credentials for this registry
     * @param token Cancellation token
     */
    getDockerLoginCredentials?(token: CancellationToken): Promise<DockerCredentials>;
}

DockerRepository:

/**
 * Interface for a Docker repository
 */
export interface DockerRepository extends RegistryTreeItem {
    /**
     * Gets all the tags for this repository.
     * @param refresh If true, a refresh is being done, and caching should not be used
     * @param token Cancellation token
     */
    getTags(refresh: boolean, token: CancellationToken): Promise<DockerTag[]>;

    /**
     * Deletes a repository. This method is optional.
     * @param token Cancellation token
     */
    delete?(token: CancellationToken): Promise<void>;
}

DockerTag:

/**
 * Interface for a Docker tag
 */
export interface DockerTag extends RegistryTreeItem {
    /**
     * Deletes a tag. This method is optional.
     * @param token Cancellation token
     */
    delete?(token: CancellationToken): Promise<void>;
}

Custom registry provider

CustomDockerRegistryProvider:

/**
 * Interface for a custom registry provider that implements `getChildTreeItems`, to provide an arbitrary tree of nodes
 */
export interface CustomDockerRegistryProvider extends DockerRegistryProviderBase, ParentTreeItem {
}

ParentTreeItem:

/**
 * Interface for a `RegistryTreeItem` with children. Part of the `CustomDockerRegistryProvider` implementation.
 */
export interface ParentTreeItem extends RegistryTreeItem {
    /**
     * Gets the child items for this tree node
     * @param refresh If true, a refresh is being done, and caching should not be used
     * @param token Cancellation token
     */
    getChildTreeItems(refresh: boolean, token: CancellationToken): Promise<RegistryTreeItem[]>;
}

Others

DockerCredentials:

/**
 * Interface for Docker credentials, used for `docker login` commands and authenticating to registries.
 */
export interface DockerCredentials {
    /**
     * The service the credentials are for
     */
    readonly service: string;

    /**
     * The username / account name
     */
    readonly account: string;

    /**
     * The secret (password, personal access token, etc.)
     */
    readonly secret: string;
}

CommandContext:

/**
 * When context/palette commands are called on nodes under the command will be given
 * arguments in the form of: `CommandContext?`, `CommandContext[]?`
 * Where the first is the selected node, and the second is the list of selected nodes
 */
export interface CommandContext {
    /**
     * The original `RegistryTreeItem` used to create this tree node
     */
    readonly originalObject: RegistryTreeItem;
}

[karolz-ms]

Thank you, @bwateratmsft, for putting together this document. Here are my suggestions--HTH!

Overview

My assumption is this document is a design proposal and not just an invitation for brainstorming. As such it should be addressed to the person(s) who does registry provider implementation (registry extension author). It should tell them what can be accomplished, and how. What "we" (VS Code Docker extension owners) think, or need to do, is only relevant to the extent that it facilitates the job of the registry extension author. Everything else is unnecessary. Also, if a facility is necessary, don't say "we would need to create X". Say "Docker extension provides X" instead. Do not say "we most likely would include X". Say "the public API package includes X".

Optionally, the extension could also implement context and palette commands. With the correct context values on nodes, the existing context and palette commands will also work.

We should enumerate what commands will be provided by the Docker extension, and briefly describe their semantics.

Registering a registry provider

In this section I would expect to learn what specific VS Code event(s) the registry provider author will have to handle to register themselves with the Docker extension. A code snippet, or a reference to an example would be very helpful.

Provider pick list

I suggest that the spec just guarantees that if the registry provider is registered with Docker extension, the corresponding registry type will be available as a choice for the user that is attempting to connect to a Docker registry. That is all. Precisely how this will happen, what UI is involved, the manifest, and the tagging, are all user experience & implementation details that are both not fully worked out, and not very important from the registry provider perspective.

RegistryTreeItem interface

Interface for all nodes that appear in the Docker extension's explorer view for registries. This is mostly-identical to vscode.TreeItem but intentionally does not extend it, because the object returned will not be directly displayed--instead, the properties will be copied into an AzExtTreeItem from Node package vscode-azureextensionui.

I would say that RegistryTreeItem objects created by the registry provider will not be directly used to build the registry tree in VS Code UI, and that any extra data carried by the original tree objects will not be available (as context) when commands are invoked. This is essentially the same as what you are saying, but focuses on implications for the registry provider, and avoids implementation details that are subject to change.

The ID for the node. Optional. This is not the same as providerId / registryId.

So what is it? Why would a RegistryTreeItem expose this property?

The command to run when the node is left-clicked

Left-clicked? Can you elaborate?

Other interfaces

getXxx() vs getCachedXxx(): since this duality is there just to implement "Refresh" gesture, I suggest that we add refresh() method on the RegistryTreeItem and drop all the getCachedXxx() methods.

[bwateratmsft]

the manifest, and the tagging, are all user experience & implementation details that are both not fully worked out, and not very important from the registry provider perspective.

I think Mike already made the decision for us that the above (including manifest) is final. Since provider authors who want to be discovered will need to update the manifest, they need to know about it.

---

The ID for the node. Optional. This is not the same as providerId / registryId.

So what is it? Why would a RegistryTreeItem expose this property?

It would be an ID unique to the specific node, and authors may want it for contextual information when executing commands. contextValue does not uniquely identify a node, it just helps determine what should be shown when right clicking; generally nodes of the same "type" will have the same contextValue. Here's the text from the API doc:

Optional id for the tree item that has to be unique across tree. The id is used to preserve the selection and expansion state of the tree item.
If not provided, an id is generated using the tree item's label. Note that when labels change, ids will change and that selection and expansion state cannot be kept stable anymore.

---

The command to run when the node is left-clicked

Left-clicked? Can you elaborate?

Here's the text from the VSCode API doc:

The command that should be executed when the tree item is selected.

When you left-click a tree node, if this command is defined, it will be executed. Within that command you also define the arguments, if any. As an example, our OpenUrlTreeItems use this so that left-clicking them opens the browser.

---

getXxx() vs getCachedXxx(): since this duality is there just to implement "Refresh" gesture, I suggest that we add refresh() method on the RegistryTreeItem and drop all the getCachedXxx() methods.

I'm not sure I understand exactly what you mean.

[philliphoff]

I agree with @karolz-ms regarding the "cached" functions; exposing parallel sets of functions in the interface is confusing and feels like it pushes requirements onto implementations that may not have such concerns. If what we're really trying to convey is whether the user explicitly refreshed a node (vs. normal navigation), then I think that's more easily done with a flag (e.g. getXxx(isRefreshing: boolean). The simplest implementations can simply ignore that flag (at the risk of perhaps not being as efficient as possible). More sophisticated implementations could decide to cache and refresh its cache based on the flag.

[bwateratmsft]

Heheh, getXxx(isRefreshing: boolean) is essentially what I had originally (though my flag was clearCache), @karolz-ms didn't like it. 😄

TBH I like that better as well, which is why I did it that way originally. Implementers have the flexibility to cache or not, but clear instruction if they are caching when they should drop it. I think caching should be as invisible to a caller as possible. In this case, the caller needs the authority to invalidate the cache, so a simple boolean flag works. The rest can be invisible behind one method.

[philliphoff]

While I understand the reasoning behind making RegistryTreeItem mirror vscode.TreeItem (because we need to control the actual instance type, because we're building on top of another library that itself build on top of vscode.TreeItem), it feels a little awkward that commands, for example, won't get back any node context.

I wonder if we can use a model where a RegistryItem (a model type) gets a callback when we're generating its associated tree item, where it can then set/update properties like icon, label, etc. The model type will then automatically be added as context to the tree item, for use by commands, etc.

[bwateratmsft]

I wonder if we can use a model where a RegistryItem (a model type) gets a callback when we're generating its associated tree item, where it can then set/update properties like icon, label, etc. The model type will then automatically be added as context to the tree item, for use by commands, etc.

That sounds like a good candidate for something we'll implement if we actually find we have need of it. Command calls will get the AzExtTreeItem sent as an argument, so they aren't context-less, just limited to the properties we define on RegistryTreeItem.

[karolz-ms]

There is little semantic difference between having refresh() method on some registry hierarchy interface vs. having isRefreshing parameter for other methods on the same interface. My preference is for the former but if you want to go with the latter, so be it.

[philliphoff]

In general, I wonder whether we can make the "simple" registry provider more of a proper subset (in terms of interface implementations) of the "custom" registry provider, rather than having two completely different provider implementations (with a somewhat complex/confusing set of optional functions in that base provider interface).

What I'm thinking of is have every registry provider be a "custom" one, but provide helper/factory functions that allow extensions to create simple implementations with minimal fuss. I'm also thinking that this would help in cases where a "custom" provider is really just a simple one with, perhaps an extra layer of parent nodes. In that case, the current proposal seems to drop them into a "implement everything yourself" rather than a model where they can add their one layer, then use helpers to generate the rest.

[philliphoff]

There is little semantic difference between having refresh() method on some registry hierarchy interface vs. having isRefreshing parameter for other methods on the same interface. My preference is for the former but if you want to go with the latter, so be it.

@karolz-ms So is your suggestion to have a refresh() function that indicates to the provider that it should regenerate items on the next getXxx() function call, or that refresh() itself should itself regenerate and return the new items? The former seems a little disconnected from the return of the items; if we're making a call to get items, it seems natural to indicate in that call whether those items should be "fresh". If the latter, I don't see much difference between refresh() and getCachedXxx() except for the naming.

[karolz-ms]

refresh() as in "drop cached data"

The idea being that you can subsequently call getXxx() as needed and only first call will incur the full cost of data retrieval.

[karolz-ms]

What I'm thinking of is have every registry provider be a "custom" one, but provide helper/factory functions that allow extensions to create simple implementations with minimal fuss.

@philliphoff interesting idea, but I am worried that it might make the contract significantly more complicated

[bwateratmsft]

What I'm thinking of is have every registry provider be a "custom" one, but provide helper/factory functions that allow extensions to create simple implementations with minimal fuss.

@philliphoff interesting idea, but I am worried that it might make the contract significantly more complicated

I agree; I feel like a "simple" way to just add a layer of nodes would actually be much more complicated.

In terms of implementation, here's how I plan to do Azure, which might help make more sense why I did it this way.

AzureCustomDockerRegistryProvider -> returns Subscription nodes -> returns Resource Group nodes -> Returns AzureRegistryV2 nodes, extension of DockerRegistry, which will actually be the point where we revert back into the basic structure--getChildren() will simply call getRepositories() -> AzureRepositoryV2 nodes, extension of DockerRepository, getChildren() will simply call getTags()--and so on.

With this approach you can "add a layer" of nodes, without making for a confusing implementation of the contract for those who want to stick with the basics. I like the extreme simplicity of the basic implementation--you literally cannot make something any simpler than getRegistries(), getRepositories(), getTags().

[philliphoff]

That sounds like a good candidate for something we'll implement if we actually find we have need of it. Command calls will get the AzExtTreeItem sent as an argument, so they aren't context-less, just limited to the properties we define on RegistryTreeItem.

I guess I'm just trying to avoid the awkwardness of the semantics of RegistryTreeItem, where the item their command receives won't be exactly the one their associated registry provider returns. At a minimum, we should expose a free-form "context" type property from RegistryTreeItem that guarantees commands and other callbacks will get back something specific to the provider implementation.

Something like:

export interface RegistryTreeItem<T> {
  // Mirror vscode.TreeItem properties
  
  readonly context?: T;
}

[bwateratmsft]

I guess I'm just trying to avoid the awkwardness of the semantics of RegistryTreeItem, where the item their command receives won't be exactly the one their associated registry provider returns. At a minimum, we should expose a free-form "context" type property from RegistryTreeItem that guarantees commands and other callbacks will get back something specific to the provider implementation.

Or explicitly define a property on the callback node containing their original object? The wrapping AzExtTreeItem will be holding on to the original object anyway.

[philliphoff]

With this approach you can "add a layer" of nodes, without making for a confusing implementation of the contract for those who want to stick with the basics. I like the extreme simplicity of the basic implementation--you literally cannot make something any simpler than getRegistries(), getRepositories(), getTags().

But DockerRegistry, DockerRepository, and DockerTag are all derived from RegistryTreeItem, which is a fairly lengthy interface. Does that mean every provider needs to define their own sets of labels, icons, etc.? Shouldn't we have a way for implementations to say "here's a tag name, generate a "standard" node for it"?

[bwateratmsft]

But DockerRegistry, DockerRepository, and DockerTag are all derived from RegistryTreeItem, which is a fairly lengthy interface. Does that mean every provider needs to define their own sets of labels, icons, etc.?

No, which is why almost all of the properties on RegistryTreeItem are optional.

Shouldn't we have a way for implementations to say "here's a tag name, generate a "standard" node for it"?

Yes, derive from the generic Registry V2 provider I will also be writing, and if your registry isn't V2 compliant, find a new registry 😉

[philliphoff]

So there will be a generic registry provider that as part of this API that extensions can derive from (or otherwise extend)? Can we make sure that's part of this proposal/design? Will you have to derive from that generic provider in whole or can one, for example, just take certain node types from it and reuse them from one's own provider? (Is this the what the Azure registry provider will be based on?)

[bwateratmsft]

So there will be a generic registry provider that as part of this API that extensions can derive from (or otherwise extend)? Can we make sure that's part of this proposal/design?

Yes and yes!

Will you have to derive from that generic provider in whole or can one, for example, just take certain node types from it and reuse them from one's own provider? (Is this the what the Azure registry provider will be based on?)

It will be split-up-able, which is what Azure will be doing. In fact, I'm planning on implementing two layers, basically--a generic caching provider that takes care of Memento and keytar storage, so that you don't have to think about permanent caching (or, in-memory caching, or secret storage), and then a RegistryV2 extension of that which can talk to V2 registries. Azure will be an extension of the latter with some extra layers, Docker Hub of the former. GitLab will also be the former.

[philliphoff]

It will be split-up-able, which is what Azure will be doing. In fact, I'm planning on implementing two layers, basically--a generic caching provider that takes care of Memento and keytar storage, so that you don't have to think about permanent caching (or, in-memory caching, or secret storage), and then a RegistryV2 extension of that which can talk to V2 registries. Azure will be an extension of the latter with some extra layers, Docker Hub of the former. GitLab will also be the former.

Good, I'm eager to see what that looks like and will wait before making any further comments. I feel like, once written down, it will end up being very similar to my idea of factory/helper functions that allow extensions to easily create parts of the tree they don't care to customize, which I think will make it clearer how to support both "simple" and "(semi-)custom" providers without having to support two completely different provider interfaces. (E.g. a provider simply returns a root RegistryTreeItem, either one of their own creation or a generic registry tree item, and where RegistryTreeItem then has a simple getChildren() function which returns the next layer (if any), again, likely created from the generic registry/repository/tag items.

[PavelSosin-320]

JFrog with Artifactory - used by SAP, all enterprises which use SAP, Finance, Helth services .... because its security is approved by certain agencies.
Kubernetes Clusters used in Cloud and on-premise infrastructure have a mechanism to store needed credentials in a secure manner based on Docker secrets with Kubernetes and Docker namespace isolation.
DockerHub is not for everybody!
Secured and Non-secured registry lists are separated in Docker daemon configuration with no intersection!

[bwateratmsft]

Closing since there hasn't been much engagement. We're planning to work on #869 this semester.