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 support for TransparentProxy.DialedDirectly #533

Merged
merged 2 commits into from
Jun 17, 2021

Conversation

lkysow
Copy link
Member

@lkysow lkysow commented Jun 14, 2021

  • renamed types.go => shared_type.go to better reflect what that
    file contains.
  • Added DialedDirectly field to TransparentProxy struct
  • Renamed CatalogDestinationsOnly to MeshDestinationsOnly to match
    Consul renaming (Rename CatalogDestinationsOnly consul#10397)
  • Added tests to validate for ProxyDefaults since that config entry now
    has more fields. The validate methods were technically tested through
    ServiceDefaults tests but I added them to ProxyDefaults since they both
    rely on that shared validation.
  • Fixed validation for OutboundListenerPort so that it returned the port
    as the error rather than the entire TransparentProxy struct.
  • Updated Consul api package to latest
  • Updated controller-runtime to 0.6.0 because I was getting an error on
    0.5.0.
  • Regenerated YAML CRDs

How I've tested this PR:

How I expect reviewers to test this PR:

Checklist:

  • Tests added
  • CHANGELOG entry added (HashiCorp engineers only, community PRs should not add a changelog entry)

@lkysow lkysow force-pushed the lkysow/dialed-directly branch 3 times, most recently from b7dfc08 to 8f0a394 Compare June 14, 2021 21:42
Copy link
Member Author

@lkysow lkysow left a comment

Choose a reason for hiding this comment

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

This will work once Consul is released that has the MeshServicesOnly field name changes.

@@ -162,7 +162,7 @@ ifeq (, $(shell which controller-gen))
CONTROLLER_GEN_TMP_DIR=$$(mktemp -d) ;\
cd $$CONTROLLER_GEN_TMP_DIR ;\
go mod init tmp ;\
go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.5.0 ;\
go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.0 ;\
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the error I was getting:

make ctrl-manifests
/Users/lkysow/go/bin/controller-gen "crd:trivialVersions=true,allowDangerousTypes=true" rbac:roleName=manager-role webhook paths="./..." output:crd:artifacts:config=config/crd/bases
github.com/hashicorp/consul-k8s/api/v1alpha1:-: conflicting types in allOf branches in schema: string vs object
Error: not all generators ran successfully
run `controller-gen crd:trivialVersions=true,allowDangerousTypes=true rbac:roleName=manager-role webhook paths=./... output:crd:artifacts:config=config/crd/bases -w` to see all available markers, or `controller-gen crd:trivialVersions=true,allowDangerousTypes=true rbac:roleName=manager-role webhook paths=./... output:crd:artifacts:config=config/crd/bases -h` for usage
make: *** [ctrl-manifests] Error 1

input *ProxyDefaults
expectedErrMsg string
}{
"meshgateway.mode": {
Copy link
Member Author

Choose a reason for hiding this comment

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

these cases are copied from servicedefaults where they share the same fields

* renamed `types.go` => `shared_type.go` to better reflect what that
file contains.
* Added `DialedDirectly` field to `TransparentProxy` struct
* Renamed `CatalogDestinationsOnly` to `MeshDestinationsOnly` to match
Consul renaming (hashicorp/consul#10397)
* Added tests to validate for ProxyDefaults since that config entry now
has more fields. The validate methods were technically tested through
ServiceDefaults tests but I added them to ProxyDefaults since they both
rely on that shared validation.
* Fixed validation for OutboundListenerPort so that it returned the port
as the error rather than the entire TransparentProxy struct.
* Updated Consul api package to latest
* Updated controller-runtime to 0.6.0 because I was getting an error on
0.5.0.
* Regenerated YAML CRDs
@thisisnotashwin thisisnotashwin force-pushed the lkysow/dialed-directly branch 2 times, most recently from e779707 to 72edde4 Compare June 17, 2021 14:26
Copy link
Contributor

@thisisnotashwin thisisnotashwin left a comment

Choose a reason for hiding this comment

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

NICE!

@thisisnotashwin thisisnotashwin force-pushed the lkysow/dialed-directly branch 2 times, most recently from 5c0ebd7 to b7a7d94 Compare June 17, 2021 14:45
@thisisnotashwin
Copy link
Contributor

I did a go get sigs.k8s.io/controller-tools/cmd/controller-gen@v0.6.0 in the project folder and it bumped up some of the dependencies in the go.mod so i checked them in. I can revert those changes if we don't want to update some of those dependencies.

@thisisnotashwin thisisnotashwin marked this pull request as ready for review June 17, 2021 14:57
@thisisnotashwin thisisnotashwin merged commit acfc3b2 into master Jun 17, 2021
@thisisnotashwin thisisnotashwin deleted the lkysow/dialed-directly branch June 17, 2021 15:19
ndhanushkodi pushed a commit to ndhanushkodi/consul-k8s that referenced this pull request Jul 9, 2021
…s for the gateway deployment templates (hashicorp#533)

* update resource settings for the gateways for lifecycle sidecar and init containers
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.

None yet

2 participants