Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add registry provider client and integration tests #404

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 47 additions & 5 deletions helper_test.go
Expand Up @@ -720,23 +720,24 @@ func createRegistryModuleWithVersion(t *testing.T, client *Client, org *Organiza
}

func createRunTask(t *testing.T, client *Client, org *Organization) (*RunTask, func()) {
runTaskURL := os.Getenv("TFC_RUN_TASK_URL")
if runTaskURL == "" {
t.Error("Cannot create a run task with an empty URL. You must set TFC_RUN_TASK_URL for run task related tests.")
}

var orgCleanup func()

if org == nil {
org, orgCleanup = createOrganization(t, client)
}

runTaskURL := os.Getenv("TFC_RUN_TASK_URL")
if runTaskURL == "" {
t.Error("Cannot create a run task with an empty URL. You must set TFC_RUN_TASK_URL for run task related tests.")
}

ctx := context.Background()
r, err := client.RunTasks.Create(ctx, org.Name, RunTaskCreateOptions{
Name: "tst-" + randomString(t),
URL: runTaskURL,
Category: "task",
})

if err != nil {
t.Fatal(err)
}
Expand All @@ -754,6 +755,47 @@ func createRunTask(t *testing.T, client *Client, org *Organization) (*RunTask, f
}
}

func createRegistryProvider(t *testing.T, client *Client, org *Organization, registryName RegistryName) (*RegistryProvider, func()) {
var orgCleanup func()

if org == nil {
org, orgCleanup = createOrganization(t, client)
}

ctx := context.Background()

if registryName != PrivateRegistry && registryName != PublicRegistry {
t.Fatal("Registry name must be either public or private")
}

namespaceName := String("tst-namespace-" + randomString(t))
if registryName == PrivateRegistry {
namespaceName = &org.Name
}

options := RegistryProviderCreateOptions{
Name: String("tst-name-" + randomString(t)),
Namespace: namespaceName,
RegistryName: &registryName,
}
prv, err := client.RegistryProviders.Create(ctx, org.Name, options)
if err != nil {
t.Fatal(err)
}

return prv, func() {
if err := client.RegistryProviders.Delete(ctx, org.Name, prv.RegistryName, prv.Namespace, prv.Name); err != nil {
t.Errorf("Error destroying registry provider! WARNING: Dangling resources\n"+
"may exist! The full error is shown below.\n\n"+
"Registry Provider: %s/%s\nError: %s", prv.Namespace, prv.Name, err)
}

if orgCleanup != nil {
orgCleanup()
}
}
}

func createSSHKey(t *testing.T, client *Client, org *Organization) (*SSHKey, func()) {
var orgCleanup func()

Expand Down
243 changes: 243 additions & 0 deletions registry_provider.go
@@ -0,0 +1,243 @@
package tfe

import (
"context"
"errors"
"fmt"
"net/url"
)

// Compile-time proof of interface implementation.
var _ RegistryProviders = (*registryProviders)(nil)

// RegistryProviders describes all the registry provider related methods that the Terraform
// Enterprise API supports.
//
// TFE API docs: https://www.terraform.io/docs/cloud/api/providers.html
type RegistryProviders interface {
// List all the providers within an organization.
List(ctx context.Context, organization string, options *RegistryProviderListOptions) (*RegistryProviderList, error)

// Create a registry provider
Create(ctx context.Context, organization string, options RegistryProviderCreateOptions) (*RegistryProvider, error)

// Read a registry provider
Read(ctx context.Context, organization string, registryName RegistryName, namespace string, name string, options *RegistryProviderReadOptions) (*RegistryProvider, error)

// Delete a registry provider
Delete(ctx context.Context, organization string, registryName RegistryName, namespace string, name string) error
}

// registryProviders implements RegistryProviders.
type registryProviders struct {
client *Client
}

// RegistryName represents which registry is being targeted
type RegistryName string

// List of available registry names
const (
PrivateRegistry RegistryName = "private"
PublicRegistry RegistryName = "public"
)

// RegistryProvider represents a registry provider
type RegistryProvider struct {
ID string `jsonapi:"primary,registry-providers"`
Namespace string `jsonapi:"attr,namespace"`
Name string `jsonapi:"attr,name"`
RegistryName RegistryName `jsonapi:"attr,registry-name"`
Permissions RegistryProviderPermissions `jsonapi:"attr,permissions"`
CreatedAt string `jsonapi:"attr,created-at"`
UpdatedAt string `jsonapi:"attr,updated-at"`

// Relations
Organization *Organization `jsonapi:"relation,organization"`
RegistryProviderVersions []RegistryProviderVersion `jsonapi:"relation,registry-provider-version"`
}

type RegistryProviderPermissions struct {
CanDelete bool `jsonapi:"attr,can-delete"`
}

type RegistryProviderListOptions struct {
ListOptions
// A query string to filter by registry_name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We follow the doc convention: // Optional: A query string to filter by registry_name even though the field being a pointer is implying that it's optional.

RegistryName *RegistryName `url:"filter[registry_name],omitempty"`
// A query string to filter by organization
OrganizationName *string `url:"filter[organization_name],omitempty"`
// A query string to do a fuzzy search
Search *string `url:"q,omitempty"`
}

type RegistryProviderList struct {
*Pagination
Items []*RegistryProvider
}

func (o RegistryProviderListOptions) valid() error {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

valid() methods go all the way in the bottom of the file ⏬
For context, file should be organized in this order:

  1. Interface/struct definitions
  2. Endpoint methods
  3. Valid and other helper methods

return nil
}

func (r *registryProviders) List(ctx context.Context, organization string, options *RegistryProviderListOptions) (*RegistryProviderList, error) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unnecessary whitespace ✏️

if !validStringID(&organization) {
return nil, ErrInvalidOrg
}
if options != nil {
if err := options.valid(); err != nil {
return nil, err
}
}

u := fmt.Sprintf("organizations/%s/registry-providers", url.QueryEscape(organization))
req, err := r.client.newRequest("GET", u, options)
if err != nil {
return nil, err
}

pl := &RegistryProviderList{}
err = r.client.do(ctx, req, pl)
if err != nil {
return nil, err
}

return pl, nil
}

// RegistryProviderCreateOptions is used when creating a registry provider
type RegistryProviderCreateOptions struct {
// Type is a public field utilized by JSON:API to
// set the resource type via the field tag.
// It is not a user-defined value and does not need to be set.
// https://jsonapi.org/format/#crud-creating
Type string `jsonapi:"primary,registry-providers"`

Namespace *string `jsonapi:"attr,namespace"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These fields should not be pointers unless the field is a struct type or the API accepts a null value (and perhaps that may be so since omitempty is omitted?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In PR #313 they are not pointers.

Name *string `jsonapi:"attr,name"`
RegistryName *RegistryName `jsonapi:"attr,registry-name"`
}

func (o RegistryProviderCreateOptions) valid() error {
if !validString(o.Name) {
return ErrRequiredName
}
if !validStringID(o.Name) {
return ErrInvalidName
}
if !validString(o.Namespace) {
return errors.New("namespace is required")
}
if !validStringID(o.Namespace) {
return errors.New("invalid value for namespace")
}
if !validString((*string)(o.RegistryName)) {
return errors.New("registry-name is required")
}
return nil
}

func (r *registryProviders) Create(ctx context.Context, organization string, options RegistryProviderCreateOptions) (*RegistryProvider, error) {
if !validStringID(&organization) {
return nil, ErrInvalidOrg
}
if err := options.valid(); err != nil {
return nil, err
}

u := fmt.Sprintf(
"organizations/%s/registry-providers",
url.QueryEscape(organization),
)
req, err := r.client.newRequest("POST", u, &options)
if err != nil {
return nil, err
}
prv := &RegistryProvider{}
err = r.client.do(ctx, req, prv)
if err != nil {
return nil, err
}

return prv, nil
}

type RegistryProviderReadOptions struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty struct? 👻

}

func (r *registryProviders) Read(ctx context.Context, organization string, registryName RegistryName, namespace string, name string, options *RegistryProviderReadOptions) (*RegistryProvider, error) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whenever I see a long method signature like that I wonder if we can use a struct to keep the signature concise? An example:

type RegistryModuleID struct {
// The organization the module belongs to, see RegistryModule.Organization.Name
Organization string
// The name of the module, see RegistryModule.Name
Name string
// The module's provider, see RegistryModule.Provider
Provider string
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is refactored in the next PR. Wondering if it's simpler to go with the large PR that has everything?

if !validStringID(&organization) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All these validStringID() checks should probably be refactored into a valid() function and perhaps should live in the ReadOptionsStruct instead of as method params (back to my previous point). What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been refactored.

return nil, ErrInvalidOrg
}
if !validString(&name) {
return nil, ErrRequiredName
}
if !validStringID(&name) {
return nil, ErrInvalidName
}
if !validString(&namespace) {
return nil, errors.New("namespace is required")
}
if !validStringID(&namespace) {
return nil, errors.New("invalid value for namespace")
}
if !validString((*string)(&registryName)) {
return nil, errors.New("registry-name is required")
}

u := fmt.Sprintf(
"organizations/%s/registry-providers/%s/%s/%s",
url.QueryEscape(organization),
url.QueryEscape(string(registryName)),
url.QueryEscape(namespace),
url.QueryEscape(name),
)
req, err := r.client.newRequest("GET", u, options)
if err != nil {
return nil, err
}

prv := &RegistryProvider{}
err = r.client.do(ctx, req, prv)
if err != nil {
return nil, err
}

return prv, nil
}

func (r *registryProviders) Delete(ctx context.Context, organization string, registryName RegistryName, namespace string, name string) error {
if !validStringID(&organization) {
return ErrInvalidOrg
}
if !validString(&name) {
return ErrRequiredName
}
if !validStringID(&name) {
return ErrInvalidName
}
if !validString(&namespace) {
return errors.New("namespace is required")
}
if !validStringID(&namespace) {
return errors.New("invalid value for namespace")
}
if !validString((*string)(&registryName)) {
return errors.New("registry-name is required")
}

u := fmt.Sprintf(
"organizations/%s/registry-providers/%s/%s/%s",
url.QueryEscape(organization),
url.QueryEscape(string(registryName)),
url.QueryEscape(namespace),
url.QueryEscape(name),
)
req, err := r.client.newRequest("DELETE", u, nil)
if err != nil {
return err
}

return r.client.do(ctx, req, nil)
}