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

Merge updates from ocm-plus repo #16

Merged

Conversation

JustinKuli
Copy link
Member

Brings in commits up to the current "release-2.4" branch from the other repo.

ckandag and others added 29 commits June 23, 2021 16:04
* lease api updates

Signed-off-by: ckandag <ckandaga@redhat.com>

* fix for kind test

Signed-off-by: ckandag <ckandaga@redhat.com>

* fix quote

Signed-off-by: ckandag <ckandaga@redhat.com>

* debug

Signed-off-by: ckandag <ckandaga@redhat.com>

* fix hubconfig logic

Signed-off-by: ckandag <ckandaga@redhat.com>

* fix fmt

Signed-off-by: ckandag <ckandaga@redhat.com>
…-cluster-management-io#133)

* change the way compliance is handled across multiple namespaces

Signed-off-by: Gus Parvin <gparvin@redhat.com>

* organize changes more from review comments

Signed-off-by: Gus Parvin <gparvin@redhat.com>

* re-organize checks based on review comments

Signed-off-by: Gus Parvin <gparvin@redhat.com>
Signed-off-by: ckandag <ckandaga@redhat.com>
…t-io#140)

* workaround to not add extras

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fix sort failure

Signed-off-by: Will Kutler <wkutler@redhat.com>

* add fix and test

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fmt

Signed-off-by: Will Kutler <wkutler@redhat.com>

* clean up

Signed-off-by: Will Kutler <wkutler@redhat.com>

* cleanup 2

Signed-off-by: Will Kutler <wkutler@redhat.com>

* remove extra compliancetypes

Signed-off-by: Will Kutler <willkutler@gmail.com>
Signed-off-by: ckandag <ckandaga@redhat.com>
Signed-off-by: Gus Parvin <gparvin@redhat.com>
* sort template in merge fn

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fmt

Signed-off-by: Will Kutler <wkutler@redhat.com>

* test logs

Signed-off-by: Will Kutler <wkutler@redhat.com>

* logs 2

Signed-off-by: Will Kutler <wkutler@redhat.com>

* clean up logs and fix test

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fix list compare

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fmt

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fix mustonlyhave

Signed-off-by: Will Kutler <wkutler@redhat.com>

* remove logs

Signed-off-by: Will Kutler <wkutler@redhat.com>

* add test to verify fix

Signed-off-by: Will Kutler <wkutler@redhat.com>

* rename function

Signed-off-by: Will Kutler <wkutler@redhat.com>
* Refactor mergeArrays

Adjusted conditionals with equivalent logic, renamed a variable.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>

* Remove dead code

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
* add comments 1/2

Signed-off-by: Will Kutler <wkutler@redhat.com>

* add comments 2/2

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fmt

Signed-off-by: Will Kutler <wkutler@redhat.com>

* comment

Signed-off-by: Will Kutler <wkutler@redhat.com>

* add new block to merge

Signed-off-by: Will Kutler <wkutler@redhat.com>

* remove unnecessary sort

Signed-off-by: Will Kutler <wkutler@redhat.com>

* remove unnecessary sort 2

Signed-off-by: Will Kutler <wkutler@redhat.com>
The "templates" package was split out to its own Go module in:
stolostron/go-template-utils#2

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
…o#149)

* try removing merge block

Signed-off-by: Will Kutler <wkutler@redhat.com>

* add e2e test

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fix list handling when len(existing) < len(template)

Signed-off-by: Will Kutler <wkutler@redhat.com>

* remove redundant mustonlyhave branch

Signed-off-by: Will Kutler <wkutler@redhat.com>
* Add test for whitespace in list

Currently, if an item in a list has whitespace at the beginning or end,
it won't get matched by the same item because only one of them is
trimmed. As a result, and enforce policy with an item like that will
keep appending duplicates of the item on the list indefinitely. This
test demonstrates the issue with a Deployment.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>

* Stop trimming some items in lists

This should resolve the issue described in the previous commit.

Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
Signed-off-by: Dale Haiducek <dhaiduce@redhat.com>
Signed-off-by: Will Kutler <wkutler@redhat.com>
This updates the dependency on the go-template-utils module.

Resolves stolostron/backlog#14952

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
Signed-off-by: Will Kutler <wkutler@redhat.com>
…agement-io#160)

This fixes the situation where the controller is comparing what is in
the policy versus what already exists, and the object being compared
contains an integer.

The code in the mergeSpecs function marshals and unmarshals the existing
object and the object in the policy. Presumably this is to create a copy
of the objects before modification. By doing so with the encoding/json
package, it converts any integers to float64.

This caused an issue in the checkFieldsWithSort function which compared
the existing object with the merged object, since the existing object
referenced here did not go through the marshal and unmarshal process,
but the merged object did. This meant that the integers in the merged
object were of type float64, but they remained integers in the existing
object. When checkFieldsWithSort converts both integers to strings to
compare, if the one of type float64 is a large enough number, it gets
printed in scientific notation where as the one that remained of type
integer does not. This would cause a mismatch even though it should have
matched.

The solution is quite simple. Just start using the json package from
Kubernetes so that the same unmarshaling process occurs in all cases.

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
Signed-off-by: ckandag <ckandaga@redhat.com>
This makes the code consistent with the policy propagator in:
stolostron/governance-policy-propagator#101

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
This is to include the fix from:
stolostron/go-template-utils#18

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
This fixes a regression as described in:
stolostron/go-template-utils#19

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
…ment-io#166)

Deep copy the policy before passing it to handleObjectTemplates since even
though handleObjectTemplates accepts a copy (i.e. not a pointer) of the
policy, policy.Spec.ObjectTemplates is a slice of pointers, so any
modifications to the objects in that slice will be reflected in
the PolicyMap cache, which can have unintended side effects.

This manifested itself when a policy has templates which when resolved,
still have intentional template delimiters (i.e. {{ and }}) in the result.
On the first time being processed, everything would go well. On the
second or third time, when the policy cache was not updated again by the
reconciler, the policy in the cache already had resolved templates. This
would lead to the templates code trying to reprocess the already
resolved policy.

Additionally to this, this caused some updates to referenced resource
objects in templates to not get properly updated in the policy.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2007575

Signed-off-by: mprahl <mprahl@users.noreply.github.com>
* Annotation to disable templates

Signed-off-by: ckandag <ckandaga@redhat.com>

* fix glog formatting

Signed-off-by: ckandag <ckandaga@redhat.com>
…o#170)

* store api resource list in case of failure

Signed-off-by: Will Kutler <wkutler@redhat.com>

* fmt

Signed-off-by: Will Kutler <wkutler@redhat.com>

* ignore error if partial list is returned

Signed-off-by: Will Kutler <wkutler@redhat.com>
Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
@JustinKuli
Copy link
Member Author

/hold t o merge manually

Copy link
Member

@dhaiducek dhaiducek left a comment

Choose a reason for hiding this comment

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

Wow! Thanks for tackling this! I looked pretty quickly and noted one thing that stuck out. I'm hoping the tests are capturing anything I might have missed.

cmd/manager/main.go Outdated Show resolved Hide resolved
Signed-off-by: Justin Kulikauskas <jkulikau@redhat.com>
@openshift-ci
Copy link

openshift-ci bot commented Nov 8, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dhaiducek, JustinKuli

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [JustinKuli,dhaiducek]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@JustinKuli JustinKuli merged commit 41e440b into open-cluster-management-io:main Nov 8, 2021
@JustinKuli JustinKuli deleted the update-upstream branch July 25, 2024 13:41
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.

6 participants