* generate krshaped logic
* add logic to the tag
* Update injection/README.md
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* move var to reconciler
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Split KResource into a file. Include an interface.
* Update apis/duck/v1/kresource_type.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* type
* rename to KRShaped
* Create common logic for bumping obsgen
* fix typo
* remove interface cast
* missed a spot
* Update to use ObjectMetaAccessor
* switch alias to duckv1
* fix var names
* include a reason
* Switch to preprocess
* rename variable from new to resource
* Create a table style test
* missed a spot
* update deps
* fix nits
* fix gomod deps
* update with new KRshaped
* Update reconciler/reconcile_common_test.go
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Update reconciler/reconcile_common_test.go
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Update reconciler/reconcile_common_test.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
Co-authored-by: Matt Moore <mattmoor@vmware.com>
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Split KResource into a file. Include an interface.
* Update apis/duck/v1/kresource_type.go
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* type
* rename to KRShaped
Co-authored-by: Matt Moore <mattmoor@vmware.com>
* Skip doing any work if Also receives no errors
1. if no errors are passed skip doing stuff (checks, allocations, etc)
2. if there's one error passed and it's empty (the same)
- this is a quite popular pattern where in API validation we do
`x.Also(validateY())` and if that succeeded then it'll be an empty
error. Which is in reality most of the time.
3. Improve tests by better printing
4. better test coverage shows commutativity of Also(a, b) and
Also(a).Also(b)
* fix flow
* Remove unused code.
* Use raw strings to avoid escaping.
* Remove unneeded type conversions.
* Preallocate slices where possible.
* Use semantic equality in psbinding reconciler.
* Some fixes we did in server upstream to pkg
- reduce formatting calls where they are not necessary
- remove casts where they are not necessary
- switch to consts where possible.
* nit
* 1more
* Make informer cache initializations retriable on error
If the first initialization fails, allow the informer cache
initialization to get retried at a later point instead of caching the
error.
* Update cache tests
* Rename 'result' type to 'informerCache'
* Create IsInDelete context
* Set up context with WithinDelete
* Test for callback delete
* fix subresource update
* Return oldObj for deletes
* include delete in webhook config
* include delete in unit test
* don't log on delete
* Add new callback pattern to pkg
* include the context
* typo
* Remove the empty instance of unstructured
* initialize the unstructured var
* Eliminate the unneeded pointer
* Pass a pointer to unstructured callback
* Create a validation specific context struct
* Move callback tests to own unit test case
* Switch from converting to decoding
* Update webhook/resourcesemantics/validation/validation.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* don't wrap context and include params
* split validation files
* include 2020 copyright
* include unit test for WithKubeClient
* Don't bother updating copyright date
* Inclue a unit test for panic
* Move dryRun to context
* Include context dry run unit test
* put the request operation in the context
* eliminate circular dep
* move kubeclient test out of context_test
* dont bother iterating callback map
* Callback takes a list of supported verbs
* Remove extra type
* Ensure Callback interface is public
* Alias Operation into validation
* alias Operation right in Webhook
* Update webhook/resourcesemantics/validation/validation_admit.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit_test.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit_test.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* Update webhook/resourcesemantics/validation/validation_admit_test.go
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
* correct parens
* minor style fixes
* Rename Callback to Func
* Fix build error
* Switch callback to take a list with a factory
* keep descriptive names
* update comment
* Drop pointer, correct comments
* Add a unit test to disallow duplicate verbs
* fix comments, struct{} for set
* switch to variadic arg for NewCallback
Co-authored-by: Victor Agababov <vagababov@gmail.com>
* Do not set to unknown status when all dependents passed unhappy check
`accessor.GetConditions()` in `findUnhappyDependent()` gets status
filed from runtime. So, if an object has following status:
```
status:
address:
url: http://hello-example.default.svc.cluster.local
conditions:
- lastTransitionTime: "2020-04-02T09:49:03Z"
status: "True"
type: AllTrafficAssigned
- lastTransitionTime: "2020-04-02T09:49:03Z"
message: autoTLS is not enabled
reason: AutoTLSNotEnabled
severity: Info
status: "True"
type: CertificateProvisioned
- lastTransitionTime: "2020-04-02T09:49:05Z"
status: "True"
type: IngressReady
- lastTransitionTime: "2020-04-02T09:49:05Z"
status: "True"
type: Ready
```
`accessor.GetConditions()` returns AllTrafficAssigned,
CertificateProvisioned, IngressReady (and Ready).
Then, current code (`len(r.dependents) != len(conditions)`) sets to
Unknown if all conditions are not unhappy. However, it does not work
if we added a new dependent and downgraded the cluster because
the number of dependents is less than current status conditions.
If all dependents are verified, it should not set to unknown
status. So, this patch changed it.
* Add unit test
* genreconciler:nonNamespaced
* Adding a force-kinds flag to the reconciler generator to allow us to generate reconcilers for non-owned types.
* force-kinds --> force-genreconciler-kinds
* some minor nits with the generators for non-knative types needed to be removed.
* remove whitespace
Co-authored-by: Nacho Cano <nachoacano@gmail.com>
* Add MarkTrueWithReason function to ConditionManager
When using `MarkTrue()` function, it does not allow us to add the reason and message.
Also, if we use `SetCondition()` instead, it does not update the happy condtion.
So, this PR adds new `MarkTrueWithReason(reason, message)` function.
* Make test better
* Add unit test for MarkTrueWithReason
* Rename markHappy with recomputeHappiness
* Realized a corner case on ordering and False > Unknown states.
* add test with unknown fall through.
* I guess some integartions allow nil happy
* add unit tests for GetMessage and GetReason
* all conditons can be nil.
* Search for unhappy dependents.
* update codegen
* Remove KResource from v1alpha1 ducks.
* looking at annotations for group overrides.
* checkpoint.
* checkpoint
* I think it is correct.
* ok
* trim down the files.
* simulate tekton
* working test out of checkin
* remove test client
* update docs
* and test.
* lint
* it ok
* update codegen
* add ability to opt-out of bindings
* I guess a year changed...
* had opt-in / opt-out backwards
* add unit test for contexts, fix comment
* clarify comments
* remove Binding from method/const names since already in bindings package
* added isUnknown in condition_set
* added IsFalse for condition set
* added GetHappyCondition
* modified comments
* modified the code based on cr
* renamed the method and removed isXXX() methods
* modified comments
Working to introduce structured logging to our tests. See #907
This work allows these test functions to be called by objects other
than *testing.T. The t.Error() calls are made compatible with
structured logging (wrapping Zap sugared logger calls) or code using
testing.T.
This PR adds facilities to make it easier to create both components of a Binding
over "Pod Spec"-able resources. Rather than rehashing it all here, please look
at `./pkg/webhook/psbinding/README.md` for more details.
* #2047 sources-controller can't recover from faulty sink (in eventing)
* #2047 sources-controller can't recover from faulty sink (in eventing)
* add unit test
* change after review
* change after review-2
Co-Authored-By: Victor Agababov <vagababov@gmail.com>
This adds a ToUnstructured method to complement the FromUnstructured method
we have had for some time. The ToUnstructured method is effectively scoped
to Knative-style types since we expect it to implement both kmeta.Accessor
and kmeta.OwnerRefable in order to ensure that the TypeMeta is always
properly populated.
This augments the injection codegen with the capability to produce a duck.InformerFactory
attached to context for each type that we process.
Now a `duck.InformerFactory` for "Addressable" can be produced by "Get"ing it from the context.
This is triggered by placing `// +genduck` on the type that implements `duck.Implementable`.
By combining our validation logic into our mutating webhook we were previously allowing for mutating webhooks evaluated after our own to modify our resources into invalid shapes. There are no guarantees around ordering of mutating webhooks (that I could find), so the only way to remedy this properly is to split apart the two into separate webhook configurations:
- `defaulting`: which runs during the mutating admission webhook phase
- `validation`: which runs during the validating admission webhook phase.
The diagram in [this post](https://kubernetes.io/blog/2019/03/21/a-guide-to-kubernetes-admission-controllers/) is very helpful in illustrating the flow of webhooks.
Fixes: https://github.com/knative/pkg/issues/847
Destination sits in a strange versioned directory without a group under APIs. Destination is in fact part of our duck type space, so it belongs under the `duck` group. This moves the definition (previously v1alpha1) to `v1` because it is referenced from source types that have been designated `v1`.
* Add IsEmpty and HTTP/HTTPS constructors, simplify tests.
* Simplify remaining construction of URLs in tests with struct initialization.
* Reverse `nonEmpty` since most cases are now non-empty.
* Update initial tests. Also fix a bug in URL.String() and update test.
* change Destination api and addressable resolver, fixed unit tests and added more unit tests
* make validateDestination visible
* rename error msg, added comments, added unit tests
* reformat error msg and change positive case order
* modified error msg when ref is not valid
* fixed typo
We have been maintaining a copy of this helper function in the Knative
Serving tests. Moving it here and adding tests so it can be used more
generally. The function allows creating a []byte patch without having to
check two levels of error conditions.
* Introduce error util ErrInvalidCombination for invalid combination
Sometimes valid value becomes invalid value by combination.
example 1. https://github.com/knative/serving/issues/5382
example 2. following combination in `spec.traffic`.
```
traffic:
- latestRevision: true
revisionName: hello-example-dk7nd
percent: 100
```
But there are no error util for them, so we need to create
custom error like c1583f3045
or `ErrInvalidValue`.
The custom error will make code complicated and `ErrInvalidValue` is
not debug friendly.
To solve it, this patch introduces an util func `ErrInvalidCombination`.
* Introduce ErrGeneric instead of ErrInvalidCombination
* Add Addressable type converters to v1 Addressable
v1alpha1 embeds v1beta1 which makes them easily convertible. However, we
cannot embed both v1beta1 and v1 into v1alpha1 easily. Updating to just
use v1 in v1alpha1 is breaking so this change implements converters
to enable moving between v1 and v1beta1/v1alpha1 without having to
implement the logic in every client needing to convert.
* Use apis.Convertible interface
* Implement RemoveCondition for ConditionManager
This implements RemoveCondition to remove a condition that matches
the condition type. Happy condition is changed appropriately if it
is a terminal and satifies one of the following conditions
1. RemovedCondition is false and happy is false
Happy can change from false to unknown or true
2. RemovedCondition is unknown and happy is unknown
Happy can change from unknown to true
* comment edit
* Addresses review comments
* Support only non terminal conditions
* Fix check
* #457 Duck type user annotation logic
* #457 Duck type user annotation logic - tests
* #457 Revert updater annotation key from lastModifier to updater
* #457 Rename HasSpec#GetSpec() to HasSpec#GetUntypedSpec()
* #457 Fix some indentation
* #457 Get group for user info annotations from the request
* #457 Reduce confusuion in webhook testing by using same group
* Add support for a structured URL type.
This type can be used to accept `url: http://asdf.com` where in code we
get a `url.URL` to interact with.
* Propagate errors parsing, cast pointer instead of copying.
* have simple tests. working on impl.
* strict setting, reflection based.
* ran codegen.
* adding license.
* update based on feedback and merge better.
* getting closer to something simpler assuming shallow reflect.
* adding validation test.
* use the json tag.
* Golang things nil typed pointers are not nil.
* Use real value of reflect invalid.
* add a missing test.
* two methods, one for update, one for single check.
* checkdep is now in apis.
* fix pkg.
* Update apis/deprecated_test.go
Co-Authored-By: n3wscott <32305648+n3wscott@users.noreply.github.com>
* add code clarity.
* include inlined struct objects recursively.
* Update commnets and add a flatten error test for inlined.
The backbone of our Condition system is our "happy" condition and
the semantics governing how other subconditions influence that condition.
When looking towards Conversion, it is possible for the set of conditions
to vary between `v1alpha1` and `v1beta1`, but the "happy" condition should
remain consistent across versions.
The main changes:
1. Provide a `ConvertTo` helper for "converting" between `duckv1beta1.Status`
types in this "lowest common denominator" sense, where we just copy the
happy condition.
2. When `InitializeConditions()` (plural) is called, seed the initial state
of sub-conditions from the initial state of the "happy" condition, if True.
This change enables us to completely change the condition space across
versions, while maintaining the consistency of the happy condition.
A couple peripheral changes:
1. Add `context.Context` to the `apis.Convertible` interface.
2. Drop `InitializeCondition()` (singular) from the `ConditionsManager` interface (I don't see any usage outside of this file).
This deprecates the `apis.Immutable` and `apis.Annotatable` interfaces,
which were both awkward niche extensions of `apis.Validatable` and
`apis.SetDefaults` for specific contexts that the former set didn't
cover well.
With this change, the expectation is that types that want to check
for immutability will instead access the "baseline" object via the
context from within updates. For example:
```
func (new *Type) Validate(ctx context.Context) *apis.FieldError {
if apis.IsInUpdate(ctx) {
old := apis.GetBaseline(ctx).(*Type)
// Update specific validation based on new and old.
}
}
```
For applying user annotations, the type writer can write:
```
func (new *Type) SetDefaults(ctx context.Context) {
if apis.IsInCreate(ctx) {
ui := apis.GetUserInfo(ctx)
// Set creator annotation from ui
}
if apis.IsInUpdate(ctx) {
ui := apis.GetUserInfo(ctx)
old := apis.GetBaseline(ctx).(*Type)
// Compare old.Spec vs. new.Spec and on changes
// update the "updater" annotation from ui.
}
}
```
One of the key motivations for this refactoring was to enable us
to do more powerful validation in `apis.Validate` beyond the niche
of immutability checking (and without introducing yet-another
one-off niche interface). In the BYO Revision name PoC I abused
`apis.Immutable` to do more arbitrary before/after validation,
which with this can simply be a part of `apis.Validatable`.
See: https://github.com/knative/serving/pull/3562
The general stance on deprecating interfaces such as these will be
to deprecate them in a non-breaking way (via a comment for now). They
will be hollowed out when the functionality is removed from the webhook,
but left in because of diamond dependency problems. In this change
we remove the `apis.Annotatable` functionality and deprecate the
`apis.Immutable` functionality.
This moves the common Condition stuff to apis, and creates a v1beta1 form of Status that uses the Condition it defines (changing this in v1alpha1 is too breaking).
There aren't really any meaningful changes in this PR, mostly reorganization. Enumerating what I did:
1. Copied `condition_set*.go` to `apis/`,
1. Copied the `Condition` portions of `conditions_types.go` to `apis/`,
1. Copied the balance of `conditions_types.go` to `apis/duck/v1beta1/status_types.go`,
1. Changed the parts of the above to reference things in the appropriate new places,
1. Removed the reflection-based `ConditionsAccessor` stuff, implementing it instead on `duckv1beta1.Status`.
1. Incorporate: https://github.com/knative/pkg/pull/358
With this, the expectation is that folks can embed the following in
the status of their resource:
```go
type FooStatus struct {
duckv1alpha1.Status `json:",inline"`
// other fields
}
```
`ObservedGeneration` is important to the usefulness of our standard
conditions because `{Ready,Succeeded}: True` is only meaningful when
`.status.observedGeneration == .metadata.generation`.
When `.status.observedGeneration != .metadata.generation` it indicates
that the controller has not yet attempted to reconcile a change in the
desired state of the world (aka `spec:`).
* Drop webhook logic to increment spec.generation
With Kubernetes 1.11+ metadata.generation now increments properly
when the status subresource is enabled on CRDs
For more details see: https://github.com/knative/serving/issues/643
* Drop the generational duck type
* Initial commit for the webhook to set the annotations about mutator.
The user that created or updated the resource will be set in the
annotations.
* update comments
* remove debug logging
* logging :/
* logging :/, returns
* logging :/ III
* error wrap
* simplify test
* rename the test
* add pkg/errors to the deps for better errors
* do not require CRD to implement Annotatable
* review issues
* fix interface as required by review
* Add `AtIndex` method to deal to attach index at the current error.
This is useful when we are dealing with primite leaf list (e.g.
a list of strings).
The alternative are
a) creating dummy error and then executing ViaFieldIndex(), which is
plain ugly
b) implementing Validate() on the primitive type via type aliasing and
then collecting errors, which seems an overkill.
In addition it seems ViaFieldIndex was not covered by unit tests,
so I've added one to take care of that.
* implement suggestions from the review
* cleanup
* cosmetic
* cosmetic
This restores the appearance of "terminal" conditions to what is was prior to the separation of "terminal" and "non-terminal" conditions.
Terminal conditions with an "Error" severity will simply omit the field.
This keeps coming up, and came to a head when both Evan and Ville bugged me about it in the same day.
Fixes: https://github.com/knative/serving/issues/2507
* Some fixes to the spoof.go and exporter.go
While reviewing some other CL, I saw some avenues for improving
spoof.go, to log the URL that's being fetched, which would help in test
debugging and to use switch construct, rather than nested if's.
While testing the change, I noticed some shifty loggin from the
exporter, so I fixed that as well while I was there.
* Continuation of the previous cleanups.
* Fix the issues with formatting by executing a grep
* and fix compilation error
* lowercase error
* fix the newly changed unit test
* duck typing - add a ConformsToType helper
Unlike VerifyType, ConformsToType will return the following:
- an error when any marshalling/unmarshalling fails
- false when the concrete type does not implement the duck type
- true when the concrete type implements the duck type
* use knative/pkg kmp to handle panics raised by go-cmp
* Some clean ups in the FieldError code and its tests.
- rename FieldError.getNormalizedError to be normalized:
- Go kind of discourages GetA() vs A()
- it's an error, so normalizing itself is also an error, so
normalizedError() is a tautology at this point
- flatten was doing a[x]=fmt.Sprintf("%s%s", a[x],y), which is a very
expensive substitution for a[x]+=y
- some minor test cleanups (fix strings, streamline `got, want:=`
pattern, etc.
* Also make normalize() and merge() operate on pointers.
As the code currently is structured we are doing lots of duplication
and copies of the objects, since we're doing []FieldError throughout
normalize() and merge() even though normalize already returns a copy of
the initial error tree, rather than a list of pointers to existing
errors.
Hence list returned by normalize() and hence merge() can actually be pointer
lists which would save us some CPU and memory.
My prior change added sorting to the duck.CreatePatch method to try and stabilize the result of jsonpatch.CreatePatch, which is otherwise non-deterministic (probably walking a map?).
My bad assumption was that the patch operations this generated wouldn't conflict, e.g. it should use `replace` vs. `remove` and `add`.
Clearly this was bad because we start getting really strange errors trying to import this into knative/serving, e.g.
https://gubernator.knative.dev/build/knative-prow/pr-logs/pull/knative_serving/2646/pull-knative-serving-integration-tests/1070435951391543298/
Immutable fields with default values may now be changed iff they change is to populate their default value. This is to support defaulting in the scenario where an object was created long ago and a new field (with a default!) is added. When controllers attempt to mutate the object status today, this would create a webhook rejection! With this change, we compare against a freshly defaulted "old" object to exclude newly defaulted fields from the immutability check.
We saw this in knative/serving for the newly added TimeoutSeconds field in Revision (otherwise immutable), which I believe it leading to upgrade testing flakes since post-upgrade Revision status updates will fail.
Victor Agababov
Weston Haught
Jason Hall
Ville Aikas
Markus Thömmes
Antoine Cotten
whaught
Evan Anderson
Kenjiro Nakayama
Scott Nichols
Ignacio (Nacho) Cano
Dave Protasowski
Matthias Wessendorf
cshou
capri-xiyue
coryrc
Matt Moore
grac3gao
Slavomir Kaslev
Adam Harwayne
Ville Aikas
mattmoor-sockpuppet
Evan Anderson
Matt Moore
Jean-Rémy Bancel
Dan Gerdesmeier
Toshinori Sugita
Laszlo Bence Nagy
Spencer Peterson
Andrew Poydence
savitaashture
Spencer Peterson
Johnu George
Jin Chi He
Ali Ok
Adriano Cunha
lIuDuI
Thomas Shafer
Zhimin Xiang
Stefan Prodan
Alex DiCarlo
Bob Corsaro
Victor Agababov
Yaron Schneider
mattmoor-sockpuppet