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

add log module #174

Closed
wants to merge 1 commit into from
Closed

add log module #174

wants to merge 1 commit into from

Conversation

shanyin
Copy link
Contributor

@shanyin shanyin commented Oct 20, 2016

add log module on the ipa-4-3 branch, the function of log module is to record any of our operation.

@MartinBasti
Copy link
Contributor

Hello, what is use case for this?

"I'm admin and I want to audit what commads were used" or what?

Have you tried, http://www.freeipa.org/page/Centralized_Logging ? Does it match your use case?

I folowing serious notes:

  • we merge new features only to master branch
  • there is no policy who can access that information, I don't think that everybody should be able to see IPA log
  • it works only for a particular replica, we have replicated topology, so commands should work for all servers
  • also parsing log everytime looks for me quite inefficient

@shanyin
Copy link
Contributor Author

shanyin commented Oct 21, 2016

Yes. As you see, our use case is I'm admin and want to audit what commads were used.

  • Because I use freeipa 4.3.1 version, adaptation through on this version, I saw the master branch source code structure is different from the ipa-4-3 branch, I am not sure if there is a problem after adding log module, so created pull request on the ipa-4-3 branch.
  • We can set up the log file permissions. Currently only the admin can view the log information in web interface.
  • The log module adapted by freeipa 4.3.1 version, not sure if there is a problem in other versions. As for the replicated topology, I need to familiar with its function.
  • Parsing the log is to show more friendly in the web interface.

BTW, there is still no respond about joining Chinese translation organization in zanata https://fedora.zanata.org/language/view/zh-CN?dswid=2727

@MartinBasti
Copy link
Contributor

Yes. As you see, our use case is I'm admin and want to audit what commads were used.

In this case, if it is only for admins I endorse you to use centralized logging:

Because I use freeipa 4.3.1 version, adaptation through on this version, I saw the master branch source code structure is different from the ipa-4-3 branch, I am not sure if there is a problem after adding log module, so created pull request on the ipa-4-3 branch.

Yes, master differs from 4.3, and as I said, new features are merged only to master, 4.3.x is only for bugfixes

We can set up the log file permissions. Currently only the admin can view the log information in web interface.

Where is this enforced in code? AFAIK you are accessing log as httpd user

The log module adapted by freeipa 4.3.1 version, not sure if there is a problem in other versions. As for the replicated topology, I need to familiar with its function.

FreeIPA supports multimaster topology, your current implementation shows only history of commands from the server where a user is actually connected. It will not provide history from all servers.

Parsing the log is to show more friendly in the web interface.

IMO Kibana and centralized logging shows log information in nice way too

BTW, there is still no respond about joining Chinese translation organization in zanata https://fedora.zanata.org/language/view/zh-CN?dswid=2727

Have you tried others way how to get into? Maybe ask teamlead on IRC
https://fedoraproject.org/wiki/L10N_Teams
https://fedoraproject.org/wiki/L10N_Simplified_Chinese_Team

@shanyin
Copy link
Contributor Author

shanyin commented Oct 25, 2016

I'm sorry didn't reply your message in time.

  • I will set up centralized logging environment later.
  • Because my environment is Ubuntu, currently the latest freeIPA version is 4.3.x, so I can only verify the log module on this version.
  • There is no restriction in code. What I mean is to control file attributes in the script after installation.
  • I will be familiar with using topological features later.

I have got the reply of the translation organization, but still in the stage of authentication. I will try to see if there is a permission to translate on freeIPA after certification.

Thank you.

@MartinBasti
Copy link
Contributor

@shanyin Did centralized logging meet your requirements?

@shanyin
Copy link
Contributor Author

shanyin commented Nov 14, 2016

I tried to use the centralized logging, but My system is Ubuntu, and the ipa-log-config tool is only supported by RHEL 7 / CentOS 7 currently. So, the centralized logging is not configured successfully.


祝:

工作顺利!生活愉快!

长沙研发中心 郑磊
电话:18684703229
邮箱:zhenglei@kylinos.cn
公司:天津麒麟信息技术有限公司
地址:湖南长沙市开福区三一大道工美大厦十四楼

------------------ Original ------------------
From: "mbasti-rh"notifications@github.com;
Date: Fri, Nov 11, 2016 07:36 PM
To: "freeipa/freeipa"freeipa@noreply.github.com;
Cc: "shanyin"zhenglei@kylinos.cn; "Mention"mention@noreply.github.com;
Subject: Re: [freeipa/freeipa] add log module (#174)

@shanyin Did centralized logging meet your requirements?


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@shanyin
Copy link
Contributor Author

shanyin commented Nov 14, 2016

@mbasti-rh I've already finished my translation basically On Zanata. The URL is https://fedora.zanata.org/webtrans/translate?project=freeipa&iteration=master&localeId=zh-CN&locale=en-US&dswid=9896#view:doc;doc:po/ipa

@MartinBasti
Copy link
Contributor

@shanyin great, I suppose you want those translations in IPA 4.4.x, so I could try to copy them from master.

@MartinBasti
Copy link
Contributor

Hello,

we agreed on devel meeting that this is not the right way how to audit/log inspection should be done with FreeIPA:

  • centralized logging is preferred solution

However we would like to merge some parts of your PR:

  • fix for missing translation strings
  • improvement of logging that might help you and can improve value of logs for users
    Would be awesome if you can send them as separate PR.

Also we endorse you to create an IPA httpd log parser as separate CLI project from this PR which may be helpful for other users as lightweight solution compared to centralized logging.

Thank you!

@MartinBasti MartinBasti added the rejected Pull Request has been rejected label Nov 14, 2016
@shanyin
Copy link
Contributor Author

shanyin commented Nov 15, 2016

What do you mean is that I should send the log codes as separate PR? If so, I will do it later.


祝:

工作顺利!生活愉快!

长沙研发中心 郑磊
电话:18684703229
邮箱:zhenglei@kylinos.cn
公司:天津麒麟信息技术有限公司
地址:湖南长沙市开福区三一大道工美大厦十四楼

------------------ Original ------------------
From: "mbasti-rh"notifications@github.com;
Date: Tue, Nov 15, 2016 01:56 AM
To: "freeipa/freeipa"freeipa@noreply.github.com;
Cc: "shanyin"zhenglei@kylinos.cn; "Mention"mention@noreply.github.com;
Subject: Re: [freeipa/freeipa] add log module (#174)

Hello,

we agreed on devel meeting that this is not the right way how to audit/log inspection should be done with FreeIPA:

centralized logging is preferred solution

However we would like to merge some parts of your PR:

fix for missing translation strings

improvement of logging that might help you and can improve value of logs for users Would be awesome if you can send them as separate PR.

Also we endorse you to create an IPA httpd log parser as separate CLI project from this PR which may be helpful for other users as lightweight solution compared to centralized logging.

Thank you!


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@shanyin
Copy link
Contributor Author

shanyin commented Nov 29, 2016

I am sorry that the follow comments could not be sent successfully.

What do you mean is that I should send the log module as separate PR? If so, I will do it later.

@MartinBasti
Copy link
Contributor

Hello,

what I meant was to send fixing of missing translations strings as separated PR and if you identified any parts of code that should be logged too, you can send a PR too.

Basically your changes in: ipalib/plugins/config.py and at the end of ipaserver/rpcserver.py (but the second one need discussion first why is that needed)

@shanyin
Copy link
Contributor Author

shanyin commented Nov 30, 2016

Hello,
I have sent fixing of missing translations as separated PR in #286.
The changes in the ipaserver/rpcserver.py file was used for parsing the apache error.log information to ipa.log that was used for providing the interfaces of Web UI log module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rejected Pull Request has been rejected
Projects
None yet
2 participants