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 app spec validate-offline (Resolves #1449) #1450

Merged

Conversation

trinitronx
Copy link
Contributor

@trinitronx trinitronx commented Oct 22, 2023

This PR implements a new sub-command, as described in #1449:

doctl  apps spec validate-offline

I followed a similar method to #225

When merged, this enables the following GitHub Actions feature to work:

Notes: My work on this PR was done as part of DigitalOcean + MLH's Hacktoberfest + Global Hack Week: Open Source

…cean#1449)

Implement a new command to validate an app spec without requiring
auth & connection to the API. This is useful for validating app specs
in CI pipelines and untrusted environments.  As there is no currently
published [YAML schema][1] for use with [`redhat.vscode-yaml`][2], this
seems to be the best approach for now.

[1]: https://www.schemastore.org/json/
[2]: https://github.com/redhat-developer/yaml-language-server
@trinitronx trinitronx changed the title Add app spec validate offline (Resolves #1449) Add app spec validate-offline (Resolves #1449) Oct 22, 2023
trinitronx added a commit to trinitronx/action-doctl that referenced this pull request Oct 23, 2023
trinitronx added a commit to trinitronx/action-doctl that referenced this pull request Oct 23, 2023
@ghost

This comment was marked as off-topic.

@ghost

This comment was marked as off-topic.

@ghost

This comment was marked as off-topic.

@ghost

This comment was marked as off-topic.

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Thanks so much for this great contribution!

You provided a lot of great context in the issue. I also prefer your original design of the --schema-only flag being used to determine if the token is needed or not. Reading over the problems you ran into, you are right that the --schema-only flag is not available when command creation happens. But we can workaround this!

We can call initServices directly if needed:

	if err := c.initServices(c); err != nil {
		return err
	}

So rather than adding a new sub-command we can use the --schema-only flag for this by:

  1. Creating the command with cmdBuilderWithInit like you did with the validate-offline one.
  2. Between the if schemaOnly block and the c.Apps().Propose call, add the call to initServices

This way, if the --schema-only flag is passed, we exit before initializing the services. If not, we still initialize the services and check for the API token before making the API call.

trinitronx added a commit to trinitronx/doctl that referenced this pull request Oct 23, 2023
trinitronx added a commit to trinitronx/doctl that referenced this pull request Oct 23, 2023
trinitronx added a commit to LyraPhase/pre-commit-digitalocean that referenced this pull request Oct 23, 2023
trinitronx added a commit to trinitronx/doctl that referenced this pull request Oct 23, 2023
@trinitronx
Copy link
Contributor Author

trinitronx commented Oct 23, 2023

Thanks so much for this great contribution!

You provided a lot of great context in the issue. I also prefer your original design of the --schema-only flag being used to determine if the token is needed or not. Reading over the problems you ran into, you are right that the --schema-only flag is not available when command creation happens. But we can workaround this!

We can call initServices directly if needed:

[...SNIP...]

@andrewsomething: Thanks for that suggestion! I've refactored the code to use that method, and tested it against some proposed changes to action-doctl. It's now tested & validated as working!

This is ready for review again 😀

P.S.: 🙏 If someone could add the Hacktoberfest issue labels once this is accepted, that would be great! Thanks!

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

👍 LGTM! Thanks again!

@andrewsomething andrewsomething merged commit 1df456e into digitalocean:main Oct 24, 2023
6 checks passed
@trinitronx trinitronx deleted the add-app-spec-validate-offline branch October 27, 2023 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setup without token for offline commands such as app spec validation
2 participants