open-horizon / anax

Horizon agent control system
https://open-horizon.github.io/docs/anax/docs/
Apache License 2.0
73 stars 98 forks source link

Clarify deploycheck message when allowPrivileged is missing #1575

Open dabooz opened 4 years ago

dabooz commented 4 years ago

When deploycheck discovers that a node and deployment policy are not compatible, it would be helpful to add clarification to the incompatibility message when the allowPrivileged constraint has been implicitly applied to the compatibility calculation. This happens when a service deployment config requests use of a privileged feature. If the message could indicate that the missing requirement was implicitly generated, it would help, for example: Existing message: "reason": { "IBM/ibm.gps_2.0.9_arm": "Policy Incompatible: Compatibility Error: Node properties do not satisfy constraint requirements. The required properties openhorizon.allowPrivileged=true were not found in the available properties version=1.0.0, ram=926, cpus=4, arch=arm, owner=varatep, batch=1, openhorizon.hardwareId=00000000bde46796, openhorizon.cpu=4, openhorizon.arch=arm, openhorizon.memory=926, openhorizon.allowPrivileged=false" } A better message would be something like: "reason": { "IBM/ibm.gps_2.0.9_arm": "Policy Incompatible: Compatibility Error: Node properties do not satisfy constraint requirements. The required property openhorizon.allowPrivileged=true was implicitly added because the service requires privileged container function. The required properties openhorizon.allowPrivileged=true were not found in the available properties version=1.0.0, ram=926, cpus=4, arch=arm, owner=varatep, batch=1, openhorizon.hardwareId=00000000bde46796, openhorizon.cpu=4, openhorizon.arch=arm, openhorizon.memory=926, openhorizon.allowPrivileged=false" }

Brun0fl commented 4 years ago

Hi how can I assign this issue to me? (this is my first time contributing to opensource)

linggao commented 4 years ago

@Brun0fl sorry, I did not see this until now. Yes, please assign it to yourself.

xiuleiyy commented 4 years ago

@linggao It seems I can not assign it to myself. What permissions should I apply for? Thanks much.

joewxboy commented 4 years ago

@Brun0fl and @acostry, once you comment on an issue, we can assign it to you. Bruno, let me know if you aren't able to get to this issue, and I can alternatively assign it to Xiulei Zhu instead.

pyrobit commented 4 years ago

@joewxboy I can do it, just figuring out the process. Thanks

joewxboy commented 4 years ago

Thanks. I'm not sure if the commit process will require a signoff, but please try to do so. The Technical Charter at section 7.b.ii states:

All new inbound code contributions must also be accompanied by a Developer Certificate of Origin (http://developercertificate.org) sign-off in the source code system that is submitted through a TSC-approved contribution process which will bind the authorized contributor and, if not self-employed, their employer to the applicable license;

We're still trying to determine if that process is enforced.

joewxboy commented 4 years ago

Using git commit -s should be an automated way to do so:

The -s option used for both alternatives causes a committer signed-off-by line to be appended to the end of the commit message body. It certifies that committer has the rights to submit this work under the same license and agrees to our Developer Certificate of Origin

sudo-panda commented 3 years ago

Is this issue still active? If yes can I take it up?

Brun0fl commented 3 years ago

Folks please feel free to re-assign this problem to @acostry, a lot of things happened in since our last contact and right now I am not feeling confident enough to contribute on this project. Att., Bruno Fonseca De Lima E-mail: brunofl@br.ibm.com IS - Cloud Infrastructure +55-51-995117649 ITIL V3 Lean Six Sigma Black Belt trained
Let's have conversations that count
            ----- Original message -----From: Baidyanath Kundu notifications@github.comTo: open-horizon/anax anax@noreply.github.comCc: Bruno de Lima brunofl@br.ibm.com, Mention mention@noreply.github.comSubject: [EXTERNAL] Re: [open-horizon/anax] Clarify deploycheck message when allowPrivileged is missing (#1575)Date: Sun, Feb 7, 2021 5:17 PM    Is this issue still active? If yes can I take it up? —You are receiving this because you were mentioned.Reply to this email directly, view it on GitHub, or unsubscribe.  

linggao commented 3 years ago

@joewxboy ^^

joewxboy commented 3 years ago

@sudo-panda Please go ahead. Reach out to @dabooz with any technical questions.

sudo-panda commented 3 years ago

@dabooz Is it necessary to maintain the order of the message?

I ask this because the message, Compatibility Error: Node properties do not satisfy constraint requirements. is generated is in policy/policy_file.go while the implicit addition of openhorizon.allowPrivileged=true is added much before in the pipeline in compcheck/policy_check.go.

So unless there is a way to differentiate between implicitly added and explicitly added openhorizon.allowPrivileged=true I don't think we can add that to policy/policy_file.go and we need to add it to the error returned at line 259 of compcheck/policy_check.go.

Also I am tracing it by starting from PolicyCompatible function in deploycheck/policy.go