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

Fixes #15239: Generate a list of directives with their id and mode #2321

Conversation

VinceMacBuche
Copy link
Member

@VinceMacBuche VinceMacBuche force-pushed the arch_15239/generate_a_list_of_directives_with_their_id_and_mode branch from 6b9dfeb to a36bd7b Compare July 19, 2019 13:28
@VinceMacBuche
Copy link
Member Author

Commit modified

val csvContent = for {
policy <- policies
} yield {
s"""${policy.id.directiveId.value};${policy.policyMode.getOrElse(policyMode.mode).name};${policy.technique.generationMode.name};${policy.technique.agentConfig.runHooks.nonEmpty};${policy.technique.id.name};${policy.technique.id.version}"""
Copy link
Member

Choose a reason for hiding this comment

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

What is going to be displayed to user for them to choose? It can't be the uuid, user won't understand. I believe it must be directive name set in UI, or an ID for that.
Also, it need to be sorted by some user interesting thing. I don't know if it's technique/directive, or just directive, or rule/directive... But someone must find it back quickly (even in that file, be it just to minimize changes between run and so that we can compare file by diff).
Also, we need to know if the directive is system (and perhaps just ignore them totally, I'm not sure we want to let the user only run one of them).
Finally, I think we need to group directive that we merged - I'm not sure it's the case at that point, but perhaps it is, I don't remember

@VinceMacBuche VinceMacBuche force-pushed the arch_15239/generate_a_list_of_directives_with_their_id_and_mode branch from a36bd7b to c7a7d5a Compare July 19, 2019 14:26
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche VinceMacBuche force-pushed the arch_15239/generate_a_list_of_directives_with_their_id_and_mode branch from c7a7d5a to b340c18 Compare July 19, 2019 14:39
@VinceMacBuche
Copy link
Member Author

Commit modified

val csvContent = for {
policy <- policies
} yield {
s"""${policy.id.directiveId.value};${policy.policyMode.getOrElse(policyMode.mode).name};${policy.technique.generationMode.name};${policy.technique.agentConfig.runHooks.nonEmpty};${policy.technique.id.name};${policy.technique.id.version};"${policy.directiveOrder.value}""""
Copy link
Member

Choose a reason for hiding this comment

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

So, we converged with a normalized solution and still easy to parse in shell!

  • fully quoted csv, ie: `"field 1","field 2","some " in field 3","some , in last field"
  • the first fields MUST be comma-free, only the last can contain comma.
    In shell, we will use cut with the numbered fields for first columns (uuid,etc) and a cut "everything remaining" for last part.

Copy link
Member

Choose a reason for hiding this comment

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

The first fields are missing the quotes

Copy link
Member

@fanf fanf left a comment

Choose a reason for hiding this comment

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

Update CSV format as indicated please :)

@VinceMacBuche
Copy link
Member Author

PR rebased

@VinceMacBuche VinceMacBuche force-pushed the arch_15239/generate_a_list_of_directives_with_their_id_and_mode branch from b340c18 to 54fa04b Compare July 22, 2019 10:44
val csvContent = for {
policy <- policies
} yield {
s"""${policy.id.directiveId.value};${policy.policyMode.getOrElse(policyMode.mode).name};${policy.technique.generationMode.name};${policy.technique.agentConfig.runHooks.nonEmpty};${policy.technique.id.name};${policy.technique.id.version};"${policy.directiveOrder.value}""""
Copy link
Member

Choose a reason for hiding this comment

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

The first fields are missing the quotes

…mode

Fixes #15239: Generate a list of directives with their id and mode
@VinceMacBuche
Copy link
Member Author

Commit modified

@VinceMacBuche
Copy link
Member Author

last push was a push dto rebase, because it was not compiling due to change in ZioRuntime;) here is the change

policy.technique.id.version ::
policy.directiveOrder.value ::
Nil ).mkString("\"","\",\"","\"")

Copy link
Member

Choose a reason for hiding this comment

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

I believe we need the list to be sorted by directiveOrder and that we need to know if the technique is system or not.

…id and mode

Fixes #15239: Generate a list of directives with their id and mode
@VinceMacBuche
Copy link
Member Author

Commit modified

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/2321
-- Your faithful QA
Kant merge: "In law a man is guilty when he violates the rights of others. In ethics he is guilty if he only thinks of doing so."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/12715/console)

… their id and mode

Fixes #15239: Generate a list of directives with their id and mode
@VinceMacBuche
Copy link
Member Author

Commit modified

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/2321
-- Your faithful QA
Kant merge: "All our knowledge begins with the senses, proceeds then to the understanding, and ends with reason. There is nothing higher than reason."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/12717/console)

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.

3 participants