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

TLS Support #630

Merged
merged 77 commits into from
Sep 2, 2022
Merged

TLS Support #630

merged 77 commits into from
Sep 2, 2022

Conversation

Integralist
Copy link
Collaborator

No description provided.

@Integralist Integralist marked this pull request as ready for review August 26, 2022 18:45
@Integralist Integralist requested review from a team and kailan and removed request for a team August 26, 2022 18:45
Copy link
Collaborator Author

@Integralist Integralist left a comment

Choose a reason for hiding this comment

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

Just some quick points of interest for the reviewer.

Also, most of the code has the same structure so once you've seen a couple of files you might find you start to pick up the pace whilst reviewing the rest.

@@ -287,6 +287,40 @@ type Interface interface {
NewListACLEntriesPaginator(i *fastly.ListACLEntriesInput) fastly.PaginatorACLEntries
NewListDictionaryItemsPaginator(i *fastly.ListDictionaryItemsInput) fastly.PaginatorDictionaryItems
NewListServicesPaginator(i *fastly.ListServicesInput) fastly.PaginatorServices

GetCustomTLSConfiguration(i *fastly.GetCustomTLSConfigurationInput) (*fastly.CustomTLSConfiguration, error)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

All the TLS go-fastly methods we need to use to support TLS in the CLI.

@@ -48,6 +48,15 @@ import (
"github.com/fastly/cli/pkg/commands/serviceversion"
"github.com/fastly/cli/pkg/commands/shellcomplete"
"github.com/fastly/cli/pkg/commands/stats"

tlsConfig "github.com/fastly/cli/pkg/commands/tls/config"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ensure Kingpin is configured to show these new TLS commands.

@@ -0,0 +1,185 @@
package config_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-config ... commands.

@@ -0,0 +1,260 @@
package activation_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-custom activation ... commands.

@@ -0,0 +1,276 @@
package certificate_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-custom certificate ... commands.

@@ -0,0 +1,60 @@
package domain_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-custom domain ... commands.

@@ -0,0 +1,213 @@
package privatekey_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-custom private-key ... commands.

@@ -0,0 +1,262 @@
package platform_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-platform ... commands.

@@ -0,0 +1,258 @@
package subscription_test
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tests for fastly tls-subscription ... commands.

Copy link
Member

@kailan kailan left a comment

Choose a reason for hiding this comment

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

Looks good, works on my machine.

The new package-level documentation is nice. reminds me that we should do the same for some of our internal repos :)

@Integralist Integralist merged commit 83ef7b7 into main Sep 2, 2022
@Integralist Integralist deleted the integralist/tls branch September 2, 2022 13:15
@Integralist Integralist added the enhancement New feature or request label Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants