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 Zaraz #1467

Closed
wants to merge 0 commits into from
Closed

Add Zaraz #1467

wants to merge 0 commits into from

Conversation

omarmosid
Copy link

@omarmosid omarmosid commented Dec 19, 2023

This PR adds support for the Zaraz API

Description

The primary goal is to be able to terraform Zaraz (RM-17254)

The following endpoints need to be covered.

  1. /config (GET and PUT)
  2. /workflow (GET and PUT)
  3. /publish (POST)
  4. /history (GET)
  5. /export (GET)
  6. /default (GET)

Please note that these aren't publicly documented as of today, but the plan is to add that too.

Has your change been tested?

Yes. Added a unit test for every function. The API itself is stable and is used regularly via the dash.

Screenshots (if appropriate):

Types of changes

What sort of change does your code introduce/modify?

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • This change is using publicly documented in cloudflare/api-schemas
    and relies on stable APIs.

@omarmosid omarmosid marked this pull request as draft December 19, 2023 11:01
Copy link

github-actions bot commented Dec 19, 2023

changelog detected ✅

@omarmosid omarmosid marked this pull request as ready for review December 19, 2023 22:22
@omarmosid
Copy link
Author

cc: @jonnyparris

zaraz.go Outdated
"github.com/goccy/go-json"
)

type ZarazConfig = map[string]interface{}
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason why we don't opt for a struct here?

zaraz.go Outdated
Response
}

type UpdateZarazConfigParams = map[string]interface{}
Copy link
Member

Choose a reason for hiding this comment

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

this should be a struct to prevent any unknown keys from being pushed into the parameters.

zaraz.go Outdated
return recordResp, nil
}

func (api *API) UpdateZarazConfig(ctx context.Context, rc *ResourceContainer, params UpdateZarazConfigParams) (ZarazConfigResponse, error) {
Copy link
Member

Choose a reason for hiding this comment

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

you definitely want UpdateZarazConfigParams to be typed here otherwise i can put anything in here without type constraints which may lead to API errors.

zaraz.go Outdated
return response, nil
}

func (api *API) UpdateZarazWorkflow(ctx context.Context, rc *ResourceContainer, workflowToUpdate string) (ZarazWorkflowResponse, error) {
Copy link
Member

Choose a reason for hiding this comment

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

the third parameter of all methods should be the params - see https://github.com/cloudflare/cloudflare-go/blob/master/docs/conventions.md#methods for examples.

zaraz.go Outdated
return response, nil
}

func (api *API) PublishZarazConfig(ctx context.Context, rc *ResourceContainer, description string) (ZarazPublishResponse, error) {
Copy link
Member

Choose a reason for hiding this comment

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

the third parameter of all methods should be the params - see https://github.com/cloudflare/cloudflare-go/blob/master/docs/conventions.md#methods for examples.

zaraz.go Outdated

type ZarazConfigRow struct {
ID int64 `json:"id,omitempty"`
UserId string `json:"usedId,omitempty"`
Copy link
Member

Choose a reason for hiding this comment

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

ID is usually one of the words we uppercase (UserId => UserID)

zaraz.go Outdated
return response, nil
}

func (api *API) GetZarazConfigHistory(ctx context.Context, rc *ResourceContainer, params GetZarazConfigHistoryParams) ([]ZarazConfigRow, *ResultInfo, error) {
Copy link
Member

Choose a reason for hiding this comment

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

generally a "fetch all" type of method is prefixed with List to denote it is capable of returning multiple values - https://github.com/cloudflare/cloudflare-go/blob/master/docs/experimental.md#consistent-crud-method-signatures

Copy link

github-actions bot commented Jan 2, 2024

Oops! It looks like no changelog entry is attached to this PR. Please include a release note as described in https://github.com/cloudflare/cloudflare-go/blob/master/docs/changelog-process.md.

Example:

```release-note:TYPE
Release note
```

If you do not require a release note to be included, please add the workflow/skip-changelog-entry label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants