Commit Graph

77 Commits

Author SHA1 Message Date
Monis Khan 0674215ef3
Switch to go.uber.org/zap for JSON formatted logging
Signed-off-by: Monis Khan <mok@vmware.com>
2022-05-24 11:17:42 -04:00
Ryan Richard fffcb7f5b4 Update to github.com/golangci/golangci-lint/cmd/golangci-lint@v1.44.2
- Two of the linters changed their names
- Updated code and nolint comments to make all linters pass with 1.44.2
- Added a new hack/install-linter.sh script to help developers install
  the expected version of the linter for local development
2022-03-08 12:28:09 -08:00
Ryan Richard 814399324f Merge branch 'main' into upstream_access_revocation_during_gc 2022-01-14 10:49:22 -08:00
Monis Khan 9599ffcfb9
Update all deps to latest where possible, bump Kube deps to v0.23.1
Highlights from this dep bump:

1. Made a copy of the v0.4.0 github.com/go-logr/stdr implementation
   for use in tests.  We must bump this dep as Kube code uses a
   newer version now.  We would have to rewrite hundreds of test log
   assertions without this copy.
2. Use github.com/felixge/httpsnoop to undo the changes made by
   ory/fosite#636 for CLI based login flows.  This is required for
   backwards compatibility with older versions of our CLI.  A
   separate change after this will update the CLI to be more
   flexible (it is purposefully not part of this change to confirm
   that we did not break anything).  For all browser login flows, we
   now redirect using http.StatusSeeOther instead of http.StatusFound.
3. Drop plog.RemoveKlogGlobalFlags as klog no longer mutates global
   process flags
4. Only bump github.com/ory/x to v0.0.297 instead of the latest
   v0.0.321 because v0.0.298+ pulls in a newer version of
   go.opentelemetry.io/otel/semconv which breaks k8s.io/apiserver.
   We should update k8s.io/apiserver to use the newer code.
5. Migrate all code from k8s.io/apimachinery/pkg/util/clock to
   k8s.io/utils/clock and k8s.io/utils/clock/testing
6. Delete testutil.NewDeleteOptionsRecorder and migrate to the new
   kubetesting.NewDeleteActionWithOptions
7. Updated ExpectedAuthorizeCodeSessionJSONFromFuzzing caused by
   fosite's new rotated_secrets OAuth client field.  This new field
   is currently not relevant to us as we have no private clients.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-12-16 21:15:27 -05:00
Ryan Richard ca2cc40769 Add impersonationProxyServerPort to the Concierge's static ConfigMap
- Used to determine on which port the impersonation proxy will bind
- Defaults to 8444, which is the old hard-coded port value
- Allow the port number to be configured to any value within the
  range 1024 to 65535
- This commit does not include adding new config knobs to the ytt
  values file, so while it is possible to change this port without
  needing to recompile, it is not convenient
2021-11-17 13:27:59 -08:00
Monis Khan 4bf715758f
Do not rotate impersonation proxy signer CA unless necessary
This change fixes a copy paste error that led to the impersonation
proxy signer CA being rotated based on the configuration of the
rotation of the aggregated API serving certificate.  This would lead
to occasional "Unauthorized" flakes in our CI environments that
rotate the serving certificate at a frequent interval.

Updated the certs_expirer controller logs to be more detailed.

Updated CA common names to be more specific (this does not update
any previously generated CAs).

Signed-off-by: Monis Khan <mok@vmware.com>
2021-10-06 12:03:49 -04:00
Monis Khan 91c8f747f4
certauthority: tolerate larger clock skew between API server and pinniped
This change updates our certificate code to use the same 5 minute
backdate that is used by the Kubernetes controller manager.  This
helps to account for clock skews between the API servers and the
kubelets that are running the pinniped pods.  While this backdating
reflects a large percentage of the lifetime of our short lived
certificates (100% for the 5 minute client certificates), even a 10
minute irrevocable client certificate is within our limits.  When
we move to the CSR based short lived certificates, they will always
have at least a 15 minute lifetime (5 minute backdating plus 10 minute
minimum valid duration).

Signed-off-by: Monis Khan <mok@vmware.com>
2021-09-21 09:32:24 -04:00
Monis Khan c356710f1f
Add leader election middleware
Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-20 12:18:25 -04:00
Monis Khan 7a812ac5ed
impersonatorconfig: only unload dynamiccert when proxy is disabled
In the upstream dynamiccertificates package, we rely on two pieces
of code:

1. DynamicServingCertificateController.newTLSContent which calls
   - clientCA.CurrentCABundleContent
   - servingCert.CurrentCertKeyContent
2. unionCAContent.VerifyOptions which calls
   - unionCAContent.CurrentCABundleContent

This results in calls to our tlsServingCertDynamicCertProvider and
impersonationSigningCertProvider.  If we Unset these providers, we
subtly break these consumers.  At best this results in test slowness
and flakes while we wait for reconcile loops to converge.  At worst,
it results in actual errors during runtime.  For example, we
previously would Unset the impersonationSigningCertProvider on any
sync loop error (even a transient one caused by a network blip or
a conflict between writes from different replicas of the concierge).
This would cause us to transiently fail to issue new certificates
from the token credential require API.  It would also cause us to
transiently fail to authenticate previously issued client certs
(which results in occasional Unauthorized errors in CI).

Signed-off-by: Monis Khan <mok@vmware.com>
2021-08-16 16:07:46 -04:00
Matt Moyer 5f679059d5
Add ClusterIP service to impersonator-config-controller informer.
Prior to this fix, this controller did not correctly react to changes to the ClusterIP service. It would still eventually react with a long delay due to our 5 minute resync interval.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-07-28 11:57:18 -05:00
Ryan Richard f17f7c0c6a Small refactors in impersonator_config.go suggested by @mattmoyer 2021-07-26 17:46:06 -07:00
Ryan Richard 708164b878 Carefully merge desired annotations into impersonation proxy Service
Don't overwrite annotations that might have come from a human user or
from some other non-Pinniped controller.
2021-07-22 17:09:50 -07:00
Matt Moyer af4cd1b515
Tolerate NotFound when deleting services in `impersonatorconfig`.
When a CredentialIssuer is switched from one service type to another (or switched to disabled mode), the `impersonatorconfig` controller will delete the previous Service, if any. Normally one Concierge pod will succeed to delete this initially and any other pods will see a NotFound error.

Before this change, the NotFound would bubble up and cause the strategy to enter a ErrorDuringSetup status until the next reconcile loop. We now handle this case without reporting an error.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-06-03 12:07:19 -05:00
Matt Moyer 01713c7ce1
Don't reconcile Service ports in impersonatorconfig.
These are tricky because a real load balancer controller (e.g., on GKE) will overwrite and set NodePort, so we can't blindly set the desired state of this fields.

For now, we will just skip reconciling these. In the future, we could be more clever about merging them together with the current state.

Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-27 17:10:25 -05:00
Matt Moyer 349d3dad83
Make temporary errors return Pending in impersonatorconfig.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-27 11:13:10 -05:00
Matt Moyer 0a47aa4843
Adjust log levels in impersonatorconfig controller.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-26 16:47:02 -05:00
Matt Moyer 1932b03c39
Refactor createOrUpdateService() method.
This updates the code to use a different mechanism for driving desired state:

- Read existing object
- If it does not exist, create desired object
- If it does exist, make a copy and set all the desired fields
- Do a deepequal to see if an update is necessary.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 15:03:04 -05:00
Matt Moyer be8118ec2e
Re-enable parallelism on TestImpersonatorConfigControllerSync.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 12:57:51 -05:00
Matt Moyer 1a4687a40a
Switch impersonatorconfig to all singleton queues.
We also no longer need an initial event, since we don't do anything unless the CredentialIssuer exists, so we'll always be triggered at the appropriate time.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 12:54:40 -05:00
Matt Moyer b13c494f93
Migrate off global logger in impersonatorconfig.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-26 12:44:05 -05:00
Margo Crawford e5a61f3b95 IPv6 address in unit tests for ClusterIPs 2021-05-26 10:30:33 -07:00
Margo Crawford f2021f1b53 Merge branch 'credentialissuer-spec-api' of github.com:vmware-tanzu/pinniped into credentialissuer-spec-api 2021-05-25 17:06:26 -07:00
Margo Crawford e2fad6932f multiple cluster ips 2021-05-25 17:01:42 -07:00
Matt Moyer 450ce6a4aa
Switch impersonatorconfig to new endpointaddr package.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-25 17:44:25 -05:00
Margo Crawford 5de9bac4ac Oof... good I wrote an integration test because that's not how updating works!
Now updating the existing service in kubernetes but with the new
annotations
2021-05-24 09:41:49 -07:00
Margo Crawford 150e879a68 Add tests for deleting services 2021-05-21 13:47:06 -07:00
Margo Crawford b4bb0db6e5 Refactor some shared code between load balancer and cluster ip creation 2021-05-21 09:57:46 -07:00
Margo Crawford 4606f1d8bd More error handling for cluster ip 2021-05-20 16:21:10 -07:00
Margo Crawford 62651eddb0 Took care of some impersonation cluster ip related todos 2021-05-20 11:57:07 -07:00
Matt Moyer ec25259901
Update impersonatorconfig controller to use new CredentialIssuer update helper.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
2021-05-20 12:26:07 -05:00
Margo Crawford 63c39454f6 WIP on impersonation clusterip service 2021-05-19 17:00:28 -07:00
Margo Crawford 9e61640c92 LoadBalancerIP updated dynamically 2021-05-19 14:16:15 -07:00
Margo Crawford 0b66321902 Changes to make the linter pass 2021-05-19 11:05:35 -07:00
Matt Moyer 297a484948
Add more validation and update tests for impersonationProxy as pointer.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-19 12:42:31 -05:00
Margo Crawford 94c370ac85 Annotations for impersonation load balancer 2021-05-18 16:54:59 -07:00
Margo Crawford eaea3471ec Validation for service type none and external endpoint none
Also added a few more test cases for provisioning a load balancer
2021-05-18 13:50:52 -07:00
Matt Moyer 4a785e73e6
WIP fixing impersonatorconfig tests 2021-05-18 14:54:04 -05:00
Margo Crawford 51f1a0ec13 WIP: not using impersonator.config just credentialissuer directly
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-18 12:16:27 -07:00
Matt Moyer 18ccf11905 Update impersonatorconfig controller to use CredentialIssuer API instead of ConfigMap.
Signed-off-by: Margo Crawford <margaretc@vmware.com>
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-05-18 09:50:35 -07:00
Margo Crawford 8b6fe0ac70 Fix lint error 2021-03-30 14:53:26 -07:00
Margo Crawford d47603472d Do not error when trying to delete the TLS secret and you get a not found 2021-03-30 14:44:06 -07:00
Margo Crawford 3742719427 Add annotation to make the idle timeout be over 1 hour rather than 1 minute
- Note that 4000 seconds is the maximum value that AWS allows.
2021-03-30 09:12:34 -07:00
Monis Khan 205c22ddbe
impersonator config: catch panics when running impersonator
Signed-off-by: Monis Khan <mok@vmware.com>
2021-03-18 10:28:28 -04:00
Monis Khan 00694c9cb6
dynamiccert: split into serving cert and CA providers
Signed-off-by: Monis Khan <mok@vmware.com>
2021-03-15 12:24:07 -04:00
Ryan Richard c82f568b2c certauthority.go: Refactor issuing client versus server certs
We were previously issuing both client certs and server certs with
both extended key usages included. Split the Issue*() methods into
separate methods for issuing server certs versus client certs so
they can have different extended key usages tailored for each use
case.

Also took the opportunity to clean up the parameters of the Issue*()
methods and New() methods to more closely match how we prefer to call
them. We were always only passing the common name part of the
pkix.Name to New(), so now the New() method just takes the common name
as a string. When making a server cert, we don't need to set the
deprecated common name field, so remove that param. When making a client
cert, we're always making it in the format expected by the Kube API
server, so just accept the username and group as parameters directly.
2021-03-12 16:09:37 -08:00
Monis Khan 2d28d1da19
Implement all optional methods in dynamic certs provider
Signed-off-by: Monis Khan <mok@vmware.com>
2021-03-11 16:24:08 -05:00
Monis Khan 7b1ecf79a6
Fix race between err chan send and re-queue
Signed-off-by: Monis Khan <mok@vmware.com>
2021-03-11 10:13:29 -05:00
Monis Khan 6582c23edb Fix a race detector error in a unit test
Signed-off-by: Ryan Richard <richardry@vmware.com>
2021-03-10 11:24:42 -08:00
Ryan Richard 0b300cbe42 Use TokenCredentialRequest instead of base64 token with impersonator
To make an impersonation request, first make a TokenCredentialRequest
to get a certificate. That cert will either be issued by the Kube
API server's CA or by a new CA specific to the impersonator. Either
way, you can then make a request to the impersonator and present
that client cert for auth and the impersonator will accept it and
make the impesonation call on your behalf.

The impersonator http handler now borrows some Kube library code
to handle request processing. This will allow us to more closely
mimic the behavior of a real API server, e.g. the client cert
auth will work exactly like the real API server.

Signed-off-by: Monis Khan <mok@vmware.com>
2021-03-10 10:30:06 -08:00
Matt Moyer 8c0a073cb6
Fix this constant name to match its value.
Signed-off-by: Matt Moyer <moyerm@vmware.com>
2021-03-08 13:31:16 -06:00