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

Some values in configuration file are being ignored #4

Open
ahussey-redhat opened this issue Jun 24, 2022 · 10 comments · Fixed by #5
Open

Some values in configuration file are being ignored #4

ahussey-redhat opened this issue Jun 24, 2022 · 10 comments · Fixed by #5

Comments

@ahussey-redhat
Copy link

ahussey-redhat commented Jun 24, 2022

I changed set;

[global]
message = test classification
foreground = #D82D2E
background = #FFFFFF
show_bottom = False
sys_info = False

but both show_bottom and sys_info are being displayed. While the message, foreground and background values are being honoured.

@redhatrises
Copy link
Member

Thanks for the report.

What version are you using?
Just to make sure that I understand, show_bottom and sys_info are not working, correct?

@ahussey-redhat
Copy link
Author

ahussey-redhat commented Jun 28, 2022

Hi Gabe,
Sorry about the delay.
I'm using version 1.7.0-14.
That is correct. Apart from setting the classification text, foreground and background colours, those are the only other changes I have made and don't seem to be honoured.

Specifying them as command line arguments, --hide-bottom is honoured, but not specifying --system-info still doesn't seem to be (as in, hostname and username are still being displayed without the flag being set).

Should --enable-spanning span the banner across primary and secondary monitors? Because that doesn't seem to be functioning with cli or config file arguments either.

GNOME Version: 42.2
Windowing System: X11
Kernel Version: 5.16.12-300.fc36.x86_64
Release: Fedora release 36 (Thirty Six)

Cheers,
Alex

@redhatrises redhatrises linked a pull request Jun 29, 2022 that will close this issue
@redhatrises
Copy link
Member

@ahussey-redhat Just merged some changes into main. Can you verify that it solves the problem for you?

@jegeland
Copy link

jegeland commented Jul 22, 2022

@redhatrises I've applied your changes to the following system and its working as expected 👍

$ uname -r
3.10.0-1160.49.1.el7.x86_64

$ cat /etc/centos-release
CentOS Linux release 7.9.2009

To be fair... I pulled down the srpm for version 1.6.7 from centos.pkgs.org and essentially applied your changes as a patch, recompiled the rpm and deployed it. Long way of saying it works fine on CentOS 7.9

I think this ticket can be closed now

@jegeland
Copy link

jegeland commented Jul 24, 2022

I have also tested it on Rocky 8:

[root@rocky8 classification-banner]# uname -r
4.18.0-372.9.1.el8.x86_64
[root@rocky8 classification-banner]# cat /etc/centos-release 
Rocky Linux release 8.6 (Green Obsidian)

Gnome Version 3.32.2

using latest from main branch (commit: c836bca) and setting Show Bottom = False does indeed not show the bottom banner.

jegeland pushed a commit to millennialsoftware/classification-banner that referenced this issue Jul 25, 2022
If the options are not specified in the config file, then the values are taken from the defaults dict. Since strtobool is used, the values are expected to be strings.

Fixes SecurityCentral#4.
@redhatrises
Copy link
Member

Thanks for reporting back. Will work on submitting these updates for rebuild into EPEL.

@jegeland
Copy link

@redhatrises

TBH, I've seen some buggy behavior in the EL8 implementation of this, especially after updating to the python3 gtk/gdk packages. I'm not a GTK developer by any means but I have been able to get the old 1.6.7 (EL7) package to work with some patches on other air-gapped environments. I have a fork going over at millennial software with what I believe to be a fully function version based on the 1.6.7 tag with some cherry-picked commits and some updates I've made (esc_timeout and making spanning actually work over multiple monitors).

I have it configured for tito, so a yum-builddep classification-banner.spec && tito build --test --rpm should give you a fully functioning RPM. I'd be happy to discuss how we move forward here.

I think the EL8 stuff and changes can be killed off since there is the gnome-extension and really this project just supports those still working on EL7 systems. Thoughts?

@ahussey-redhat
Copy link
Author

ahussey-redhat commented Jul 26, 2022

Sorry about my delay in response. This year has been full on.
I am primarily testing this on Fedora 36.

I will try and test your changes on F36 soon

@jegeland , you are correct. Using the gnome-shell-extension-classification-banner is the preferred method

@redhatrises
Copy link
Member

@redhatrises

TBH, I've seen some buggy behavior in the EL8 implementation of this, especially after updating to the python3 gtk/gdk packages. I'm not a GTK developer by any means but I have been able to get the old 1.6.7 (EL7) package to work with some patches on other air-gapped environments. I have a fork going over at millennial software with what I believe to be a fully function version based on the 1.6.7 tag with some cherry-picked commits and some updates I've made (esc_timeout and making spanning actually work over multiple monitors).

There is a maint-1.6 branch that we could use to merge in the necessary cherry-picks, bump the release to 1.6.8, and rebuild for EPEL.

I have it configured for tito, so a yum-builddep classification-banner.spec && tito build --test --rpm should give you a fully functioning RPM. I'd be happy to discuss how we move forward here.

Nice. But not sure that tito is used as a part of getting the package in the epel repositories which would probably be important to get the rpm updated.

I think the EL8 stuff and changes can be killed off since there is the gnome-extension and really this project just supports those still working on EL7 systems. Thoughts?

I wouldn't go that far. Not everyone uses Gnome and not everyone will use the extension for various reasons.

@jegeland
Copy link

jegeland commented Aug 1, 2022

There is a maint-1.6 branch that we could use to merge in the necessary cherry-picks, bump the release to 1.6.8, and rebuild for EPEL.

Roger that, I'll get a PR going this week for review on the maint-1.6 branch to get the changes I verified in this current issue plus the updates I've made.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants