From 9b25a65ae8c3a06c93ba34631ca3cabd8ce06075 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 21 Jun 2023 22:06:50 +0200 Subject: [PATCH 01/41] Change dbus API to fit new proposal --- rust/agama-dbus-server/src/questions.rs | 172 ++++++++++-------------- rust/agama-lib/src/lib.rs | 2 +- rust/agama-lib/src/questions.rs | 89 ++++++++++++ 3 files changed, 161 insertions(+), 102 deletions(-) create mode 100644 rust/agama-lib/src/questions.rs diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index ca7bb6c93e..635301d9d5 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -1,141 +1,67 @@ use std::collections::HashMap; use crate::error::Error; -use agama_lib::connection_to; +use log; +use agama_lib::{connection_to,questions::{self, GenericQuestion}}; use anyhow::Context; use zbus::{dbus_interface, fdo::ObjectManager, zvariant::ObjectPath, Connection}; -/// Basic generic question that fits question without special needs #[derive(Clone, Debug)] -pub struct GenericQuestion { - /// numeric id used to indetify question on dbus - id: u32, - /// Textual representation of question. Expected to be read by people - text: String, - /// possible answers for question - options: Vec, - /// default answer. Can be used as hint or preselection - default_option: String, - /// Confirmed answer. If empty then not answered yet. - answer: String, -} - -impl GenericQuestion { - pub fn new(id: u32, text: String, options: Vec, default_option: String) -> Self { - Self { - id, - text, - options, - default_option, - answer: String::from(""), - } - } - - pub fn object_path(&self) -> String { - format!("/org/opensuse/Agama/Questions1/{}", self.id) - } -} +struct GenericQuestionObject(questions::GenericQuestion); #[dbus_interface(name = "org.opensuse.Agama.Questions1.Generic")] -impl GenericQuestion { +impl GenericQuestionObject { #[dbus_interface(property)] pub fn id(&self) -> u32 { - self.id + self.0.id } #[dbus_interface(property)] pub fn text(&self) -> &str { - self.text.as_str() + self.0.text.as_str() } #[dbus_interface(property)] pub fn options(&self) -> Vec { - self.options.to_owned() + self.0.options.to_owned() } #[dbus_interface(property)] pub fn default_option(&self) -> &str { - self.default_option.as_str() + self.0.default_option.as_str() } #[dbus_interface(property)] pub fn answer(&self) -> &str { - &self.answer + &self.0.answer } #[dbus_interface(property)] pub fn set_answer(&mut self, value: &str) -> Result<(), zbus::fdo::Error> { // TODO verify if answer exists in options or if it is valid in other way - self.answer = value.to_string(); + self.0.answer = value.to_string(); Ok(()) } } -/// Specialized question for Luks partition activation -#[derive(Clone, Debug)] -pub struct LuksQuestion { - /// Luks password. Empty means no password set. - password: String, - /// number of previous attempts to decrypt partition - attempt: u8, - /// rest of question data that is same as for other questions - generic_question: GenericQuestion, -} - -impl LuksQuestion { - fn device_info(device: &str, label: &str, size: &str) -> String { - let mut result = device.to_string(); - if !label.is_empty() { - result = format!("{} {}", result, label); - } - - if !size.is_empty() { - result = format!("{} ({})", result, size); - } - - result - } - - pub fn new(id: u32, device: String, label: String, size: String, attempt: u8) -> Self { - let msg = format!( - "The device {} is encrypted.", - Self::device_info(device.as_str(), label.as_str(), size.as_str()) - ); - let base = GenericQuestion::new( - id, - msg, - vec!["skip".to_string(), "decrypt".to_string()], - "skip".to_string(), - ); - - Self { - password: "".to_string(), - attempt, - generic_question: base, - } - } - - pub fn base(&self) -> &GenericQuestion { - &self.generic_question - } -} +struct LuksQuestionObject(questions::LuksQuestion); #[dbus_interface(name = "org.opensuse.Agama.Questions1.LuksActivation")] -impl LuksQuestion { +impl LuksQuestionObject { #[dbus_interface(property)] pub fn password(&self) -> &str { - self.password.as_str() + self.0.password.as_str() } #[dbus_interface(property)] pub fn set_password(&mut self, value: &str) -> () { - self.password = value.to_string(); + self.0.password = value.to_string(); } #[dbus_interface(property)] pub fn attempt(&self) -> u8 { - self.attempt + self.0.attempt } } @@ -145,10 +71,24 @@ enum QuestionType { Luks, } +trait AnswerStrategy { + /// TODO: find way to be able to answer any type of question, not just generic + fn answer(&self, question: &GenericQuestion) -> Option; +} + +struct DefaultAnswers; + +impl AnswerStrategy for DefaultAnswers { + fn answer(&self, question: &GenericQuestion) -> Option { + return Some(question.default_option.clone()) + } +} + pub struct Questions { questions: HashMap, connection: Connection, last_id: u32, + answer_strategies: Vec> } #[dbus_interface(name = "org.opensuse.Agama.Questions1")] @@ -157,25 +97,31 @@ impl Questions { #[dbus_interface(name = "New")] async fn new_question( &mut self, + class: &str, text: &str, options: Vec<&str>, - default_option: Vec<&str>, + default_option: &str, + data: HashMap ) -> Result { let id = self.last_id; self.last_id += 1; // TODO use some thread safety let options = options.iter().map(|o| o.to_string()).collect(); // TODO: enforce default option and do not use array for it to avoid that unwrap - let question = GenericQuestion::new( + let mut question = questions::GenericQuestion::new( id, + class.to_string(), text.to_string(), options, - default_option.first().unwrap().to_string(), + default_option.to_string(), + data, ); + self.fill_answer(&mut question); let object_path = ObjectPath::try_from(question.object_path()).unwrap(); + let question_object = GenericQuestionObject(question); self.connection .object_server() - .at(question.object_path(), question) + .at(object_path.clone(), question_object) .await?; self.questions.insert(id, QuestionType::Generic); Ok(object_path) @@ -188,28 +134,34 @@ impl Questions { label: &str, size: &str, attempt: u8, + data: HashMap, ) -> Result { let id = self.last_id; self.last_id += 1; // TODO use some thread safety - let question = LuksQuestion::new( + let question = questions::LuksQuestion::new( id, + "storage.encryption.activation".to_string(), device.to_string(), label.to_string(), size.to_string(), attempt, + data ); - let object_path = ObjectPath::try_from(question.base().object_path()).unwrap(); + let object_path = ObjectPath::try_from(question.base.object_path()).unwrap(); - let base = question.base().clone(); + let mut base_question = question.base.clone(); + self.fill_answer(&mut base_question); + let base_object = GenericQuestionObject(base_question); + self.connection .object_server() - .at(base.object_path(), question) + .at(object_path.clone(), LuksQuestionObject(question)) .await?; // NOTE: order here is important as each interface cause signal, so frontend should wait only for GenericQuestions // which should be the last interface added self.connection .object_server() - .at(base.object_path(), base) + .at(object_path.clone(), base_object) .await?; self.questions.insert(id, QuestionType::Luks); @@ -225,23 +177,30 @@ impl Questions { QuestionType::Generic => { self.connection .object_server() - .remove::(question.clone()) + .remove::(question.clone()) .await?; } QuestionType::Luks => { self.connection .object_server() - .remove::(question.clone()) + .remove::(question.clone()) .await?; self.connection .object_server() - .remove::(question.clone()) + .remove::(question.clone()) .await?; } }; self.questions.remove(&id); Ok(()) } + + /// sets questions to be answered by default answer instead of asking user + async fn use_default_answer(&mut self) -> Result<(), Error> { + log::info!("Answer questions with default option"); + self.answer_strategies.push(Box::new(DefaultAnswers{})); + Ok(()) + } } impl Questions { @@ -252,6 +211,17 @@ impl Questions { questions: HashMap::new(), connection: connection.to_owned(), last_id: 0, + answer_strategies: vec![], + } + } + + /// tries to provide answer to question using answer strategies + fn fill_answer(&self, question: &mut GenericQuestion) -> () { + for strategy in self.answer_strategies.iter() { + match strategy.answer(question) { + None => (), + Some(answer) => question.answer = answer, + } } } } diff --git a/rust/agama-lib/src/lib.rs b/rust/agama-lib/src/lib.rs index 4ae2bd00d2..09c8dd2a73 100644 --- a/rust/agama-lib/src/lib.rs +++ b/rust/agama-lib/src/lib.rs @@ -13,7 +13,7 @@ pub mod progress; pub mod proxies; mod store; pub use store::Store; - +pub mod questions; use crate::error::ServiceError; use anyhow::Context; diff --git a/rust/agama-lib/src/questions.rs b/rust/agama-lib/src/questions.rs new file mode 100644 index 0000000000..ad48e8afb5 --- /dev/null +++ b/rust/agama-lib/src/questions.rs @@ -0,0 +1,89 @@ +use std::collections::HashMap; + +/// module holdings data model for agama questions + +/// Basic generic question that fits question without special needs +#[derive(Clone, Debug)] +pub struct GenericQuestion { + /// numeric id used to indetify question on dbus + pub id: u32, + /// class of questions. Similar kinds of questions share same class. + /// It is dot separated list of elements. Examples are + /// `storage.luks.actication` or `software.repositories.unknown_gpg` + pub class: String, + /// Textual representation of question. Expected to be read by people + pub text: String, + /// possible answers for question + pub options: Vec, + /// default answer. Can be used as hint or preselection and it is used as answer for unattended questions. + pub default_option: String, + /// additional data to help identify questions. Useful for automatic answers. It is question specific. + pub data: HashMap, + /// Confirmed answer. If empty then not answered yet. + pub answer: String, +} + +impl GenericQuestion { + pub fn new(id: u32, class: String, text: String, options: Vec, default_option: String, data: HashMap) -> Self { + Self { + id, + class, + text, + options, + default_option, + data, + answer: String::from(""), + } + } + + pub fn object_path(&self) -> String { + format!("/org/opensuse/Agama/Questions1/{}", self.id) + } +} + +/// Specialized question for Luks partition activation +#[derive(Clone, Debug)] +pub struct LuksQuestion { + /// Luks password. Empty means no password set. + pub password: String, + /// number of previous attempts to decrypt partition + pub attempt: u8, + /// rest of question data that is same as for other questions + pub base: GenericQuestion, +} + +impl LuksQuestion { + fn device_info(device: &str, label: &str, size: &str) -> String { + let mut result = device.to_string(); + if !label.is_empty() { + result = format!("{} {}", result, label); + } + + if !size.is_empty() { + result = format!("{} ({})", result, size); + } + + result + } + + pub fn new(id: u32, class: String, device: String, label: String, size: String, attempt: u8, data: HashMap) -> Self { + let msg = format!( + "The device {} is encrypted.", + Self::device_info(device.as_str(), label.as_str(), size.as_str()) + ); + let base = GenericQuestion::new( + id, + class, + msg, + vec!["skip".to_string(), "decrypt".to_string()], + "skip".to_string(), + data, + ); + + Self { + password: "".to_string(), + attempt, + base, + } + } +} From 7e56e5fdb5c9ee6313b81e41a0a5d67ec575d280 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 27 Jun 2023 23:08:01 +0200 Subject: [PATCH 02/41] change API to use more mixin approach --- rust/agama-dbus-server/src/questions.rs | 96 ++++++++++++++++--------- rust/agama-lib/src/questions.rs | 38 ++-------- 2 files changed, 67 insertions(+), 67 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index 635301d9d5..493501c388 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use crate::error::Error; use log; -use agama_lib::{connection_to,questions::{self, GenericQuestion}}; +use agama_lib::{connection_to,questions::{self, GenericQuestion, WithPassword}}; use anyhow::Context; use zbus::{dbus_interface, fdo::ObjectManager, zvariant::ObjectPath, Connection}; @@ -16,6 +16,16 @@ impl GenericQuestionObject { self.0.id } + #[dbus_interface(property)] + pub fn class(&self) -> &str { + &self.0.class + } + + #[dbus_interface(property)] + pub fn data(&self) -> HashMap { + self.0.data.to_owned() + } + #[dbus_interface(property)] pub fn text(&self) -> &str { self.0.text.as_str() @@ -45,10 +55,11 @@ impl GenericQuestionObject { } } -struct LuksQuestionObject(questions::LuksQuestion); +/// Mixin interface for questions that are base + contain question for password +struct WithPasswordObject(questions::WithPassword); -#[dbus_interface(name = "org.opensuse.Agama.Questions1.LuksActivation")] -impl LuksQuestionObject { +#[dbus_interface(name = "org.opensuse.Agama.Questions1.WithPassword")] +impl WithPasswordObject { #[dbus_interface(property)] pub fn password(&self) -> &str { self.0.password.as_str() @@ -58,22 +69,19 @@ impl LuksQuestionObject { pub fn set_password(&mut self, value: &str) -> () { self.0.password = value.to_string(); } - - #[dbus_interface(property)] - pub fn attempt(&self) -> u8 { - self.0.attempt - } } /// Question types used to be able to properly remove object from dbus enum QuestionType { - Generic, - Luks, + Base, + BaseWithPassword, } trait AnswerStrategy { - /// TODO: find way to be able to answer any type of question, not just generic + /// Provides answer for generic question fn answer(&self, question: &GenericQuestion) -> Option; + /// Provides answer and password for base question with password + fn answer_with_password(&self, question: &WithPassword) -> (Option, Option); } struct DefaultAnswers; @@ -82,6 +90,10 @@ impl AnswerStrategy for DefaultAnswers { fn answer(&self, question: &GenericQuestion) -> Option { return Some(question.default_option.clone()) } + + fn answer_with_password(&self, question: &WithPassword) -> (Option, Option) { + return (Some(question.base.default_option.clone()), None) + } } pub struct Questions { @@ -106,7 +118,6 @@ impl Questions { let id = self.last_id; self.last_id += 1; // TODO use some thread safety let options = options.iter().map(|o| o.to_string()).collect(); - // TODO: enforce default option and do not use array for it to avoid that unwrap let mut question = questions::GenericQuestion::new( id, class.to_string(), @@ -123,39 +134,43 @@ impl Questions { .object_server() .at(object_path.clone(), question_object) .await?; - self.questions.insert(id, QuestionType::Generic); + self.questions.insert(id, QuestionType::Base); Ok(object_path) } /// creates new specialized luks activation question without answer and password - async fn new_luks_activation( + async fn new_with_password( &mut self, - device: &str, - label: &str, - size: &str, - attempt: u8, - data: HashMap, + class: &str, + text: &str, + options: Vec<&str>, + default_option: &str, + data: HashMap ) -> Result { let id = self.last_id; self.last_id += 1; // TODO use some thread safety - let question = questions::LuksQuestion::new( + // TODO: share code better + let options = options.iter().map(|o| o.to_string()).collect(); + let base = questions::GenericQuestion::new( id, - "storage.encryption.activation".to_string(), - device.to_string(), - label.to_string(), - size.to_string(), - attempt, - data + class.to_string(), + text.to_string(), + options, + default_option.to_string(), + data, + ); + let mut question = questions::WithPassword::new( + base ); let object_path = ObjectPath::try_from(question.base.object_path()).unwrap(); - let mut base_question = question.base.clone(); - self.fill_answer(&mut base_question); + let base_question = question.base.clone(); + self.fill_answer_with_password(&mut question); let base_object = GenericQuestionObject(base_question); self.connection .object_server() - .at(object_path.clone(), LuksQuestionObject(question)) + .at(object_path.clone(), WithPasswordObject(question)) .await?; // NOTE: order here is important as each interface cause signal, so frontend should wait only for GenericQuestions // which should be the last interface added @@ -164,7 +179,7 @@ impl Questions { .at(object_path.clone(), base_object) .await?; - self.questions.insert(id, QuestionType::Luks); + self.questions.insert(id, QuestionType::BaseWithPassword); Ok(object_path) } @@ -174,20 +189,20 @@ impl Questions { let id: u32 = question.rsplit("/").next().unwrap().parse().unwrap(); let qtype = self.questions.get(&id).unwrap(); match qtype { - QuestionType::Generic => { + QuestionType::Base => { self.connection .object_server() .remove::(question.clone()) .await?; } - QuestionType::Luks => { + QuestionType::BaseWithPassword => { self.connection .object_server() .remove::(question.clone()) .await?; self.connection .object_server() - .remove::(question.clone()) + .remove::(question.clone()) .await?; } }; @@ -224,6 +239,19 @@ impl Questions { } } } + + /// tries to provide answer to question using answer strategies + fn fill_answer_with_password(&self, question: &mut WithPassword) -> () { + for strategy in self.answer_strategies.iter() { + let (answer, password) = strategy.answer_with_password(question); + if let Some(password) = password { + question.password = password; + } + if let Some(answer) = answer { + question.base.answer = answer; + } + } + } } /// Starts questions dbus service together with Object manager diff --git a/rust/agama-lib/src/questions.rs b/rust/agama-lib/src/questions.rs index ad48e8afb5..e718d8f8dd 100644 --- a/rust/agama-lib/src/questions.rs +++ b/rust/agama-lib/src/questions.rs @@ -41,48 +41,20 @@ impl GenericQuestion { } } -/// Specialized question for Luks partition activation +/// Composition for questions which include password. +/// TODO: research a bit how ideally do mixins in rust #[derive(Clone, Debug)] -pub struct LuksQuestion { +pub struct WithPassword { /// Luks password. Empty means no password set. pub password: String, - /// number of previous attempts to decrypt partition - pub attempt: u8, /// rest of question data that is same as for other questions pub base: GenericQuestion, } -impl LuksQuestion { - fn device_info(device: &str, label: &str, size: &str) -> String { - let mut result = device.to_string(); - if !label.is_empty() { - result = format!("{} {}", result, label); - } - - if !size.is_empty() { - result = format!("{} ({})", result, size); - } - - result - } - - pub fn new(id: u32, class: String, device: String, label: String, size: String, attempt: u8, data: HashMap) -> Self { - let msg = format!( - "The device {} is encrypted.", - Self::device_info(device.as_str(), label.as_str(), size.as_str()) - ); - let base = GenericQuestion::new( - id, - class, - msg, - vec!["skip".to_string(), "decrypt".to_string()], - "skip".to_string(), - data, - ); - +impl WithPassword { + pub fn new(base: GenericQuestion) -> Self { Self { password: "".to_string(), - attempt, base, } } From 21c64d658e6c3187b25778127f4f02be3bd1360b Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 29 Jun 2023 00:20:36 +0200 Subject: [PATCH 03/41] adapt ruby service to new questions API --- service/lib/agama/dbus/clients/question.rb | 8 +- service/lib/agama/dbus/clients/questions.rb | 20 ++-- service/lib/agama/luks_activation_question.rb | 92 ------------------- service/lib/agama/question.rb | 61 +++--------- service/lib/agama/question_with_password.rb | 28 ++++++ service/lib/agama/software/callbacks/media.rb | 8 +- .../lib/agama/software/callbacks/signature.rb | 16 +++- .../agama/storage/callbacks/activate_luks.rb | 37 +++++++- .../storage/callbacks/activate_multipath.rb | 7 +- .../agama/storage/callbacks/commit_error.rb | 10 +- 10 files changed, 126 insertions(+), 161 deletions(-) delete mode 100644 service/lib/agama/luks_activation_question.rb create mode 100644 service/lib/agama/question_with_password.rb diff --git a/service/lib/agama/dbus/clients/question.rb b/service/lib/agama/dbus/clients/question.rb index 0b119f0e3c..5dacccd98f 100644 --- a/service/lib/agama/dbus/clients/question.rb +++ b/service/lib/agama/dbus/clients/question.rb @@ -26,8 +26,8 @@ module DBus module Clients # D-Bus client for asking a question. class Question < Base - LUKS_ACTIVATION_IFACE = "org.opensuse.Agama.Questions1.LuksActivation" - private_constant :LUKS_ACTIVATION_IFACE + WITH_PASSWORD_IFACE = "org.opensuse.Agama.Questions1.WithPassword" + private_constant :WITH_PASSWORD_IFACE # @return [::DBus::ProxyObject] attr_reader :dbus_object @@ -39,9 +39,9 @@ def initialize(object_path) @dbus_object = service[object_path] @dbus_iface = @dbus_object["org.opensuse.Agama.Questions1.Generic"] # one D-Bus client for all kinds of questions - return unless @dbus_object.has_iface?(LUKS_ACTIVATION_IFACE) + return unless @dbus_object.has_iface?(WITH_PASSWORD_IFACE) - @luks_iface = @dbus_object[LUKS_ACTIVATION_IFACE] + @password_iface = @dbus_object[WITH_PASSWORD_IFACE] end # @return [String] diff --git a/service/lib/agama/dbus/clients/questions.rb b/service/lib/agama/dbus/clients/questions.rb index 07c86e9642..1d92f11b1e 100644 --- a/service/lib/agama/dbus/clients/questions.rb +++ b/service/lib/agama/dbus/clients/questions.rb @@ -116,8 +116,8 @@ def ask(question) # @param question [Agama::Question] # @return [::DBus::ObjectPath] def add_question(question) - if question.is_a?(Agama::LuksActivationQuestion) - add_luks_activation_question(question) + if question.is_a?(Agama::QuestionWithPassword) + add_question_with_password(question) else add_generic_question(question) end @@ -129,19 +129,25 @@ def add_question(question) # @return [::DBus::ObjectPath] def add_generic_question(question) @dbus_object.New( + question.qclass, question.text, question.options.map(&:to_s), - Array(question.default_option&.to_s) + question.default_option.to_s, + question.data ) end # Adds a question for activating LUKS # - # @param question [Agama::LuksActivationQuestion] + # @param question [Agama::QuestionWithPassword] # @return [::DBus::ObjectPath] - def add_luks_activation_question(question) - @dbus_object.NewLuksActivation( - question.device, question.label, question.size, question.attempt + def add_question_with_password(question) + @dbus_object.NewWithPassword( + question.qclass, + question.text, + question.options.map(&:to_s), + question.default_option.to_s, + question.data ) end end diff --git a/service/lib/agama/luks_activation_question.rb b/service/lib/agama/luks_activation_question.rb deleted file mode 100644 index 8c9c913c0c..0000000000 --- a/service/lib/agama/luks_activation_question.rb +++ /dev/null @@ -1,92 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require "agama/question" - -module Agama - # This class represent a question to ask whether to activate a LUKS device - # - # Clients have to answer one of these options: - # * skip: to skip the activation of the LUKS device - # * decrypt: to activate the device using the provided password - # - # @example - # question = LuksActivationQuestion.new("/dev/sda1", label: "mydata", size: "10 GiB") - # question.password = "n0ts3cr3t" - # - # question.answer = :skip # in case you do not want to activate the device - # - # question.answer = :decrypt # in case you want to decrypt with the given password - class LuksActivationQuestion < Question - # @return [String] - attr_reader :device - - # @return [String, nil] - attr_reader :label - - # @return [String, nil] - attr_reader :size - - # Current attempt to decrypt the device - # - # @return [Integer] - attr_reader :attempt - - # Constructor - # - # @param device [String] name of the device to be activated (e.g., "/dev/sda1") - # @param label [String, nil] label of the device - # @param size [String, nil] size of the device (e.g., "5 GiB") - def initialize(device, label: nil, size: nil, attempt: 1) - @device = device - @label = label - @size = size - @attempt = attempt - - super(generate_text, options: [:skip, :decrypt]) - end - - # Password to activate the LUKS device - # - # @return [String, nil] nil means a password has not been provided yet - attr_accessor :password - - private - - # Generate the text for the question - # - # @return [String] - def generate_text - "The device #{device_info} is encrypted." - end - - # Device information to include in the question - # - # @return [String] - def device_info - info = [device] - info << label unless label.to_s.empty? - info << "(#{size})" unless size.to_s.empty? - - info.join(" ") - end - end -end diff --git a/service/lib/agama/question.rb b/service/lib/agama/question.rb index 0198360e14..b44168308f 100644 --- a/service/lib/agama/question.rb +++ b/service/lib/agama/question.rb @@ -20,15 +20,16 @@ # find current contact information at www.suse.com. module Agama - # This class represents a question + # This class represents a question to be created # # Questions are used when some information needs to be asked. For example, a question could be # created for asking whether to continue or not when an error is detected. class Question - # Each question is identified by an unique id + # Class of the question + # Helps with identification of same type of questions # - # @return [Integer] - attr_reader :id + # @return [String] + attr_reader :qclass # Text of the question # @@ -44,7 +45,7 @@ class Question # Default option to use as answer # - # @return [Symbol, nil] + # @return [Symbol] attr_reader :default_option # Answer of the question @@ -52,50 +53,18 @@ class Question # @return [Symbol, nil] nil if the question is not answered yet attr_reader :answer - def initialize(text, options: [], default_option: nil) - @id = IdGenerator.next + # Additional data to hold identify question or improve UI to display it + # It is tight with {qclass}. + # + # @return [Hash] + attr_reader :data + + def initialize(qclass:, text:, options:, default_option:, data: {}) + @qclass = qclass @text = text @options = options @default_option = default_option - end - - # Answers the question with an option - # - # @raise [ArgumentError] if the given value is not a valid answer. - # - # @param value [Symbol] - def answer=(value) - raise ArgumentError, "Invalid answer. Options: #{options}" unless valid_answer?(value) - - @answer = value - end - - # Whether the question is already answered - # - # @return [Boolean] - def answered? - !answer.nil? - end - - private - - # Checks whether the given value is a valid answer - # - # @param value [Symbol] - # @return [Boolean] - def valid_answer?(value) - options.include?(value) - end - - # Helper class for generating unique ids - class IdGenerator - # Generates the next id to be used - # - # @return [Integer] - def self.next - @last_id ||= 0 - @last_id += 1 - end + @data = data end end end diff --git a/service/lib/agama/question_with_password.rb b/service/lib/agama/question_with_password.rb new file mode 100644 index 0000000000..9ac22f642a --- /dev/null +++ b/service/lib/agama/question_with_password.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Copyright (c) [2022] SUSE LLC +# +# All Rights Reserved. +# +# This program is free software; you can redistribute it and/or modify it +# under the terms of version 2 of the GNU General Public License as published +# by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along +# with this program; if not, contact SUSE LLC. +# +# To contact SUSE LLC about this file by physical or electronic mail, you may +# find current contact information at www.suse.com. + +require "agama/question" + +module Agama + # This class represent a question that contain additional field to provide password + class QuestionWithPassword < Question + end +end diff --git a/service/lib/agama/software/callbacks/media.rb b/service/lib/agama/software/callbacks/media.rb index b7c402aaa2..ae76a6df6e 100644 --- a/service/lib/agama/software/callbacks/media.rb +++ b/service/lib/agama/software/callbacks/media.rb @@ -54,10 +54,14 @@ def setup # @return [String] # @see https://github.com/yast/yast-yast2/blob/19180445ab935a25edd4ae0243aa7a3bcd09c9de/library/packages/src/modules/PackageCallbacks.rb#L620 # rubocop:disable Metrics/ParameterLists - def media_change(_error_code, error, _url, _product, _current, _current_label, _wanted, + def media_change(_error_code, error, url, _product, _current, _current_label, _wanted, _wanted_label, _double_sided, _devices, _current_device) question = Agama::Question.new( - error, options: [:Retry, :Skip], default_option: :Retry + qclass: "software.medium_error", + text: error, + options: [:Retry, :Skip], + default_option: :Skip, + data: { "url" => url } ) questions_client.ask(question) do |question_client| (question_client.answer == :Retry) ? "" : "S" diff --git a/service/lib/agama/software/callbacks/signature.rb b/service/lib/agama/software/callbacks/signature.rb index 7ceffe7455..168fadd085 100644 --- a/service/lib/agama/software/callbacks/signature.rb +++ b/service/lib/agama/software/callbacks/signature.rb @@ -69,7 +69,11 @@ def accept_unsigned_file(filename, repo_id) ) question = Agama::Question.new( - message, options: [:Yes, :No], default_option: :No + qclass: "software.unsigned_file", + text: message, + options: [:Yes, :No], + default_option: :No, + data: { "filename" => filename } ) questions_client.ask(question) do |question_client| question_client.answer == :Yes @@ -89,7 +93,15 @@ def import_gpg_key(key, _repo_id) ) question = Agama::Question.new( - message, options: [:Trust, :Skip], default_option: :Skip + qclass: "software.import_gpg", + text: message, + options: [:Trust, :Skip], + default_option: :Skip, + data: { + "id" => key["id"], + "name" => key["name"], + "fingerprint" => fingerprint + } ) questions_client.ask(question) do |question_client| diff --git a/service/lib/agama/storage/callbacks/activate_luks.rb b/service/lib/agama/storage/callbacks/activate_luks.rb index f34285578d..ac40cda07e 100644 --- a/service/lib/agama/storage/callbacks/activate_luks.rb +++ b/service/lib/agama/storage/callbacks/activate_luks.rb @@ -67,12 +67,21 @@ def call(info, attempt) # Question to ask for LUKS activation # - # @return [LuksActivationQuestion] + # @return [QuestionWithPassword] def question(info, attempt) - LuksActivationQuestion.new(info.device_name, - label: info.label, - size: formatted_size(info.size), - attempt: attempt) + data = { + "device" => info.device_name, + "label" => info.label, + "size" => formatted_size(info.size), + "attempt" => attempt + } + QuestionWithPassword.new( + qclass: "storage.luks_activation", + text: generate_text(data), + options: [:skip, :decrypt], + default_option: :decrypt, + data: data + ) end # Generates a formatted representation of the size @@ -82,6 +91,24 @@ def question(info, attempt) def formatted_size(value) Y2Storage::DiskSize.new(value).to_human_string end + + # Generate the text for the question + # + # @return [String] + def generate_text(data) + "The device #{device_info(data)} is encrypted." + end + + # Device information to include in the question + # + # @return [String] + def device_info(data) + info = [data["device"]] + info << data["label"] unless data["label"].to_s.empty? + info << "(#{data["size"]})" unless data["size"].to_s.empty? + + info.join(" ") + end end end end diff --git a/service/lib/agama/storage/callbacks/activate_multipath.rb b/service/lib/agama/storage/callbacks/activate_multipath.rb index 57b71b0971..7009204447 100644 --- a/service/lib/agama/storage/callbacks/activate_multipath.rb +++ b/service/lib/agama/storage/callbacks/activate_multipath.rb @@ -64,7 +64,12 @@ def question text = "The system seems to have multipath hardware. " \ "Do you want to activate multipath?" - Question.new(text, options: [:yes, :no]) + Question.new( + qclass: "storage.activate_multipath", + text: text, + options: [:yes, :no], + default_option: :yes + ) end end end diff --git a/service/lib/agama/storage/callbacks/commit_error.rb b/service/lib/agama/storage/callbacks/commit_error.rb index 67595838d2..fcf8c75742 100644 --- a/service/lib/agama/storage/callbacks/commit_error.rb +++ b/service/lib/agama/storage/callbacks/commit_error.rb @@ -69,11 +69,17 @@ def call(message, details) # Question to ask to continue # # @return [Question] - def question(message, _details) + def question(message, details) text = "There was an error performing the following action: #{message}. " \ "Do you want to continue with the rest of storage actions?" - Question.new(text, options: [:yes, :no]) + Question.new( + qclass: "storage.commit_error", + text: text, + options: [:yes, :no], + default_option: :no, + data: { "details" => details } + ) end end end From f889ae20140040cd1bb77801ff7111ecc8e9ebe8 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 11 Jul 2023 16:57:21 +0200 Subject: [PATCH 04/41] adapt UI and fix service --- service/lib/agama/dbus/clients/questions.rb | 2 +- .../agama/storage/callbacks/activate_luks.rb | 2 +- web/src/client/questions.js | 19 +++++++++---------- .../questions/LuksActivationQuestion.jsx | 2 +- web/src/components/questions/Questions.jsx | 12 ++++++------ 5 files changed, 18 insertions(+), 19 deletions(-) diff --git a/service/lib/agama/dbus/clients/questions.rb b/service/lib/agama/dbus/clients/questions.rb index 1d92f11b1e..81a468e603 100644 --- a/service/lib/agama/dbus/clients/questions.rb +++ b/service/lib/agama/dbus/clients/questions.rb @@ -21,7 +21,7 @@ require "agama/dbus/clients/base" require "agama/dbus/clients/question" -require "agama/luks_activation_question" +require "agama/dbus/clients/question_with_password" module Agama module DBus diff --git a/service/lib/agama/storage/callbacks/activate_luks.rb b/service/lib/agama/storage/callbacks/activate_luks.rb index ac40cda07e..2a18db4530 100644 --- a/service/lib/agama/storage/callbacks/activate_luks.rb +++ b/service/lib/agama/storage/callbacks/activate_luks.rb @@ -19,7 +19,7 @@ # To contact SUSE LLC about this file by physical or electronic mail, you may # find current contact information at www.suse.com. -require "agama/luks_activation_question" +require "agama/question_with_password" require "y2storage/disk_size" module Agama diff --git a/web/src/client/questions.js b/web/src/client/questions.js index 8babc33d3c..db9083c323 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -35,14 +35,14 @@ const DBUS_CONFIG = { question: { ifaces: { generic: "org.opensuse.Agama.Questions1.Generic", - luksActivation: "org.opensuse.Agama.Questions1.LuksActivation" + withPassword: "org.opensuse.Agama.Questions1.WithPassword" } } }; const QUESTION_TYPES = { generic: "generic", - luksActivation: "luksActivation" + withPassword: "withPassword" }; /** @@ -75,8 +75,6 @@ const fetchValue = (ifaceProperties, key) => { }; /** - * Builds a question from the given D-Bus question - * * @param {Object} dbusQuestion * @return {Object} */ @@ -93,15 +91,16 @@ function buildQuestion(dbusQuestion) { question.options = fetchValue(dbusProperties, "Options"); question.defaultOption = fetchValue(dbusProperties, "DefaultOption"); question.text = fetchValue(dbusProperties, "Text"); + question.class = fetchValue(dbusProperties, "Class"); + question.data = fetchValue(dbusProperties, "Data"); question.answer = fetchValue(dbusProperties, "Answer"); } - if (ifaces.includes(DBUS_CONFIG.question.ifaces.luksActivation)) { - const dbusProperties = ifacesAndProperties[DBUS_CONFIG.question.ifaces.luksActivation]; + if (ifaces.includes(DBUS_CONFIG.question.ifaces.withPassword)) { + const dbusProperties = ifacesAndProperties[DBUS_CONFIG.question.ifaces.withPassword]; - question.type = QUESTION_TYPES.luksActivation; + question.type = QUESTION_TYPES.withPassword; question.password = fetchValue(dbusProperties, "Password"); - question.attempt = fetchValue(dbusProperties, "Attempt"); } return question; @@ -145,8 +144,8 @@ class QuestionsClient { async answer(question) { const path = DBUS_CONFIG.questions.path + "/" + question.id; - if (question.type === QUESTION_TYPES.luksActivation) { - const proxy = await this.client.proxy(DBUS_CONFIG.question.ifaces.luksActivation, path); + if (question.type === QUESTION_TYPES.withPassword) { + const proxy = await this.client.proxy(DBUS_CONFIG.question.ifaces.withPassword, path); proxy.Password = question.password; } diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index 3e1b32f776..88044dc60c 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -58,7 +58,7 @@ export default function LuksActivationQuestion({ question, answerCallback }) { aria-label="Question" titleIconVariant={() => } > - { renderAlert(question.attempt) } + { renderAlert(question.data.attempt) } { question.text } diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index 1b9691ee87..7de0b92ceb 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -25,11 +25,6 @@ import { useCancellablePromise } from "~/utils"; import { GenericQuestion, LuksActivationQuestion } from "~/components/questions"; -const QUESTION_TYPES = { - generic: GenericQuestion, - luksActivation: LuksActivationQuestion -}; - export default function Questions() { const client = useInstallerClient(); const { cancellablePromise } = useCancellablePromise(); @@ -67,6 +62,11 @@ export default function Questions() { // Renders the first pending question const [currentQuestion] = pendingQuestions; - const QuestionComponent = QUESTION_TYPES[currentQuestion.type]; + let QuestionComponent = GenericQuestion; + // show specialized popup for luks activation question + // more can follow as it will be needed + if (currentQuestion.class === "storage.luks_activation") { + QuestionComponent = LuksActivationQuestion; + } return ; } From a25b96ee4849814103623c04fa585038609a4ed7 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 11 Jul 2023 17:12:45 +0200 Subject: [PATCH 05/41] fix file name --- service/lib/agama/dbus/clients/questions.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/agama/dbus/clients/questions.rb b/service/lib/agama/dbus/clients/questions.rb index 81a468e603..f0528e54e8 100644 --- a/service/lib/agama/dbus/clients/questions.rb +++ b/service/lib/agama/dbus/clients/questions.rb @@ -21,7 +21,7 @@ require "agama/dbus/clients/base" require "agama/dbus/clients/question" -require "agama/dbus/clients/question_with_password" +require "agama/question_with_password" module Agama module DBus From 7101ec900363f8b5642cd576e90ee04cfd745f73 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 11 Jul 2023 21:04:32 +0200 Subject: [PATCH 06/41] fix merge errors --- rust/agama-dbus-server/src/questions.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index c90848e7db..493501c388 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -38,10 +38,7 @@ impl GenericQuestionObject { #[dbus_interface(property)] pub fn default_option(&self) -> &str { - match self.default_option { - Some(ref option) => option.as_str(), - None => "", - } + self.0.default_option.as_str() } #[dbus_interface(property)] From c9ec48bb38921ffa82146aed903cf33fb5546afb Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Tue, 11 Jul 2023 22:20:04 +0200 Subject: [PATCH 07/41] fix type of attempts --- service/lib/agama/storage/callbacks/activate_luks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/agama/storage/callbacks/activate_luks.rb b/service/lib/agama/storage/callbacks/activate_luks.rb index 2a18db4530..d13d42178a 100644 --- a/service/lib/agama/storage/callbacks/activate_luks.rb +++ b/service/lib/agama/storage/callbacks/activate_luks.rb @@ -73,7 +73,7 @@ def question(info, attempt) "device" => info.device_name, "label" => info.label, "size" => formatted_size(info.size), - "attempt" => attempt + "attempt" => attempt.to_s } QuestionWithPassword.new( qclass: "storage.luks_activation", From cfc68a898ee84726aa4e281cb51631b05c142d37 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 10:54:16 +0200 Subject: [PATCH 08/41] add more logging for easier debugging --- rust/agama-lib/src/questions.rs | 3 +++ web/src/client/questions.js | 2 ++ web/src/components/questions/Questions.jsx | 2 ++ 3 files changed, 7 insertions(+) diff --git a/rust/agama-lib/src/questions.rs b/rust/agama-lib/src/questions.rs index e718d8f8dd..5216843b46 100644 --- a/rust/agama-lib/src/questions.rs +++ b/rust/agama-lib/src/questions.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use log::info; /// module holdings data model for agama questions @@ -25,6 +26,7 @@ pub struct GenericQuestion { impl GenericQuestion { pub fn new(id: u32, class: String, text: String, options: Vec, default_option: String, data: HashMap) -> Self { + info!("Creating new generic question with text {}", text); Self { id, class, @@ -53,6 +55,7 @@ pub struct WithPassword { impl WithPassword { pub fn new(base: GenericQuestion) -> Self { + info!("Adding to question with password interface."); Self { password: "".to_string(), base, diff --git a/web/src/client/questions.js b/web/src/client/questions.js index db9083c323..17658c69c3 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -84,6 +84,7 @@ function buildQuestion(dbusQuestion) { const ifacesAndProperties = getIfacesAndProperties(dbusQuestion); if (ifaces.includes(DBUS_CONFIG.question.ifaces.generic)) { + console.log("adding generic question"); const dbusProperties = ifacesAndProperties[DBUS_CONFIG.question.ifaces.generic]; question.type = QUESTION_TYPES.generic; @@ -97,6 +98,7 @@ function buildQuestion(dbusQuestion) { } if (ifaces.includes(DBUS_CONFIG.question.ifaces.withPassword)) { + console.log("adding withPassword interface"); const dbusProperties = ifacesAndProperties[DBUS_CONFIG.question.ifaces.withPassword]; question.type = QUESTION_TYPES.withPassword; diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index 7de0b92ceb..2507513975 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -63,10 +63,12 @@ export default function Questions() { // Renders the first pending question const [currentQuestion] = pendingQuestions; let QuestionComponent = GenericQuestion; + console.log(currentQuestion); // show specialized popup for luks activation question // more can follow as it will be needed if (currentQuestion.class === "storage.luks_activation") { QuestionComponent = LuksActivationQuestion; + console.log("get luks question"); } return ; } From c6e63a3e83aa2ee8cde39dcef9d4427a3e2e10ee Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 11:30:18 +0200 Subject: [PATCH 09/41] more logging --- web/src/components/questions/Questions.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index 2507513975..8c1fde281b 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -14,7 +14,7 @@ * * You should have received a copy of the GNU General Public License along * with this program; if not, contact SUSE LLC. - * + *Creating new * To contact SUSE LLC about this file by physical or electronic mail, you may * find current contact information at www.suse.com. */ From e90713295843c741865e9ef4c3efcfc737b101b5 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 13:12:36 +0200 Subject: [PATCH 10/41] more logging --- web/src/components/questions/LuksActivationQuestion.jsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index 88044dc60c..2826ffab83 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -38,6 +38,9 @@ export default function LuksActivationQuestion({ question, answerCallback }) { const conditions = { disable: { decrypt: password === "" } }; const defaultAction = "decrypt"; + console.log("luks question:"); + console.log(question); + const actionCallback = (option) => { question.password = password; question.answer = option; From fb7c2abf0fed0bd7f7d5f4a2392f0e8793805a3e Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 13:51:57 +0200 Subject: [PATCH 11/41] fix passing number of attempts --- web/src/components/questions/LuksActivationQuestion.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index 2826ffab83..abe802a305 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -61,7 +61,7 @@ export default function LuksActivationQuestion({ question, answerCallback }) { aria-label="Question" titleIconVariant={() => } > - { renderAlert(question.data.attempt) } + { renderAlert(parseInt(question.data.attempt)) } { question.text } From c38139a430f75d90b81c65a2b03c185009d5c453 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 15:42:38 +0200 Subject: [PATCH 12/41] more logging to reveal why password is not passed --- web/src/client/questions.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/src/client/questions.js b/web/src/client/questions.js index 17658c69c3..e0fb9daeda 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -144,9 +144,13 @@ class QuestionsClient { * @param {Object} question */ async answer(question) { + console.log("answering question"); const path = DBUS_CONFIG.questions.path + "/" + question.id; if (question.type === QUESTION_TYPES.withPassword) { + console.log("answering question with password"); + // TODO: REMOVE + console.log(question); const proxy = await this.client.proxy(DBUS_CONFIG.question.ifaces.withPassword, path); proxy.Password = question.password; } From 7f161dad366acc08cc18eca54ced7c2296cfca91 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 15:56:56 +0200 Subject: [PATCH 13/41] more logging to debug answering --- web/src/components/questions/LuksActivationQuestion.jsx | 2 ++ 1 file changed, 2 insertions(+) diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index abe802a305..4c5eb01593 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -42,8 +42,10 @@ export default function LuksActivationQuestion({ question, answerCallback }) { console.log(question); const actionCallback = (option) => { + console.log("answering question"); question.password = password; question.answer = option; + console.log(question) answerCallback(question); }; From f5cbfeab8cb8f72eb71abd3d68552390bfa853a7 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 17:33:30 +0200 Subject: [PATCH 14/41] fix typo --- web/src/components/questions/LuksActivationQuestion.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index 4c5eb01593..f296191a38 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -45,7 +45,7 @@ export default function LuksActivationQuestion({ question, answerCallback }) { console.log("answering question"); question.password = password; question.answer = option; - console.log(question) + console.log(question); answerCallback(question); }; From f2a22a806cd2e358a36a4f2f19668c2df80ec8b2 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 21:10:59 +0200 Subject: [PATCH 15/41] even more logging --- web/src/components/questions/LuksActivationQuestion.jsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index f296191a38..7123eecaec 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -35,6 +35,8 @@ const renderAlert = (attempt) => { export default function LuksActivationQuestion({ question, answerCallback }) { const [password, setPassword] = useState(question.password || ""); + console.log("password value"); + console.log(password); const conditions = { disable: { decrypt: password === "" } }; const defaultAction = "decrypt"; @@ -50,6 +52,9 @@ export default function LuksActivationQuestion({ question, answerCallback }) { }; const triggerDefaultAction = (e) => { + console.log("form action"); + console.log("password value"); + console.log(password); e.preventDefault(); if (!conditions.disable?.[defaultAction]) { actionCallback(defaultAction); From defc0f3566bdd02ba2cd087fb7765aebb35b23ed Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 21:30:13 +0200 Subject: [PATCH 16/41] now log also on ruby side --- service/lib/agama/storage/callbacks/activate_luks.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/service/lib/agama/storage/callbacks/activate_luks.rb b/service/lib/agama/storage/callbacks/activate_luks.rb index d13d42178a..9b18324331 100644 --- a/service/lib/agama/storage/callbacks/activate_luks.rb +++ b/service/lib/agama/storage/callbacks/activate_luks.rb @@ -50,6 +50,8 @@ def call(info, attempt) question = question(info, attempt) questions_client.ask(question) do |question_client| + # TODO: remove! + @logger.info("answer: #{question_client.answer.inspect} pwd #{question.password.inspect}") activate = question_client.answer == :decrypt password = question_client.password From 521d726f066a497e85c270b0d9572143b8c91a94 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 21:50:07 +0200 Subject: [PATCH 17/41] fix log message --- service/lib/agama/storage/callbacks/activate_luks.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/lib/agama/storage/callbacks/activate_luks.rb b/service/lib/agama/storage/callbacks/activate_luks.rb index 9b18324331..b8c07157fa 100644 --- a/service/lib/agama/storage/callbacks/activate_luks.rb +++ b/service/lib/agama/storage/callbacks/activate_luks.rb @@ -51,7 +51,7 @@ def call(info, attempt) questions_client.ask(question) do |question_client| # TODO: remove! - @logger.info("answer: #{question_client.answer.inspect} pwd #{question.password.inspect}") + @logger.info("answer: #{question_client.answer.inspect} pwd #{question_client.password.inspect}") activate = question_client.answer == :decrypt password = question_client.password From 4f8e82d4709f4e5c50a617b36c2e680f30c5bb64 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Wed, 12 Jul 2023 22:25:08 +0200 Subject: [PATCH 18/41] lets hope for final fix --- rust/agama-dbus-server/src/questions.rs | 4 +++- service/lib/agama/dbus/clients/question.rb | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index 493501c388..9968359e11 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use crate::error::Error; -use log; +use log::{self, info}; use agama_lib::{connection_to,questions::{self, GenericQuestion, WithPassword}}; use anyhow::Context; use zbus::{dbus_interface, fdo::ObjectManager, zvariant::ObjectPath, Connection}; @@ -62,11 +62,13 @@ struct WithPasswordObject(questions::WithPassword); impl WithPasswordObject { #[dbus_interface(property)] pub fn password(&self) -> &str { + info!("Reading password {}", self.0.password.as_str()); self.0.password.as_str() } #[dbus_interface(property)] pub fn set_password(&mut self, value: &str) -> () { + info!("Setting password to {}", value); self.0.password = value.to_string(); } } diff --git a/service/lib/agama/dbus/clients/question.rb b/service/lib/agama/dbus/clients/question.rb index 5dacccd98f..b0399dc63d 100644 --- a/service/lib/agama/dbus/clients/question.rb +++ b/service/lib/agama/dbus/clients/question.rb @@ -59,11 +59,11 @@ def answer @dbus_iface["Answer"].to_sym end - # @return [String,nil] Password or nil if there is no LUKS interface + # @return [String,nil] Password or nil if there is no withPassword interface def password - return nil unless @luks_iface + return nil unless @password_iface - @luks_iface["Password"] + @password_iface["Password"] end # Whether the question is already answered From ede020c5feb51d9d9a912b2497ea89e91b2c3bb6 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 07:07:26 +0200 Subject: [PATCH 19/41] fix rust formatting --- rust/agama-dbus-server/src/questions.rs | 47 +++++++++++++------------ rust/agama-lib/src/questions.rs | 13 +++++-- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index 9968359e11..c7809379bd 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -1,9 +1,12 @@ use std::collections::HashMap; use crate::error::Error; -use log::{self, info}; -use agama_lib::{connection_to,questions::{self, GenericQuestion, WithPassword}}; +use agama_lib::{ + connection_to, + questions::{self, GenericQuestion, WithPassword}, +}; use anyhow::Context; +use log::{self, info}; use zbus::{dbus_interface, fdo::ObjectManager, zvariant::ObjectPath, Connection}; #[derive(Clone, Debug)] @@ -90,11 +93,11 @@ struct DefaultAnswers; impl AnswerStrategy for DefaultAnswers { fn answer(&self, question: &GenericQuestion) -> Option { - return Some(question.default_option.clone()) + return Some(question.default_option.clone()); } fn answer_with_password(&self, question: &WithPassword) -> (Option, Option) { - return (Some(question.base.default_option.clone()), None) + return (Some(question.base.default_option.clone()), None); } } @@ -102,7 +105,7 @@ pub struct Questions { questions: HashMap, connection: Connection, last_id: u32, - answer_strategies: Vec> + answer_strategies: Vec>, } #[dbus_interface(name = "org.opensuse.Agama.Questions1")] @@ -115,7 +118,7 @@ impl Questions { text: &str, options: Vec<&str>, default_option: &str, - data: HashMap + data: HashMap, ) -> Result { let id = self.last_id; self.last_id += 1; // TODO use some thread safety @@ -147,11 +150,11 @@ impl Questions { text: &str, options: Vec<&str>, default_option: &str, - data: HashMap + data: HashMap, ) -> Result { let id = self.last_id; self.last_id += 1; // TODO use some thread safety - // TODO: share code better + // TODO: share code better let options = options.iter().map(|o| o.to_string()).collect(); let base = questions::GenericQuestion::new( id, @@ -161,15 +164,13 @@ impl Questions { default_option.to_string(), data, ); - let mut question = questions::WithPassword::new( - base - ); + let mut question = questions::WithPassword::new(base); let object_path = ObjectPath::try_from(question.base.object_path()).unwrap(); let base_question = question.base.clone(); self.fill_answer_with_password(&mut question); let base_object = GenericQuestionObject(base_question); - + self.connection .object_server() .at(object_path.clone(), WithPasswordObject(question)) @@ -215,7 +216,7 @@ impl Questions { /// sets questions to be answered by default answer instead of asking user async fn use_default_answer(&mut self) -> Result<(), Error> { log::info!("Answer questions with default option"); - self.answer_strategies.push(Box::new(DefaultAnswers{})); + self.answer_strategies.push(Box::new(DefaultAnswers {})); Ok(()) } } @@ -242,18 +243,18 @@ impl Questions { } } - /// tries to provide answer to question using answer strategies - fn fill_answer_with_password(&self, question: &mut WithPassword) -> () { - for strategy in self.answer_strategies.iter() { - let (answer, password) = strategy.answer_with_password(question); - if let Some(password) = password { - question.password = password; - } - if let Some(answer) = answer { - question.base.answer = answer; - } + /// tries to provide answer to question using answer strategies + fn fill_answer_with_password(&self, question: &mut WithPassword) -> () { + for strategy in self.answer_strategies.iter() { + let (answer, password) = strategy.answer_with_password(question); + if let Some(password) = password { + question.password = password; + } + if let Some(answer) = answer { + question.base.answer = answer; } } + } } /// Starts questions dbus service together with Object manager diff --git a/rust/agama-lib/src/questions.rs b/rust/agama-lib/src/questions.rs index 5216843b46..2737305381 100644 --- a/rust/agama-lib/src/questions.rs +++ b/rust/agama-lib/src/questions.rs @@ -1,5 +1,5 @@ -use std::collections::HashMap; use log::info; +use std::collections::HashMap; /// module holdings data model for agama questions @@ -9,7 +9,7 @@ pub struct GenericQuestion { /// numeric id used to indetify question on dbus pub id: u32, /// class of questions. Similar kinds of questions share same class. - /// It is dot separated list of elements. Examples are + /// It is dot separated list of elements. Examples are /// `storage.luks.actication` or `software.repositories.unknown_gpg` pub class: String, /// Textual representation of question. Expected to be read by people @@ -25,7 +25,14 @@ pub struct GenericQuestion { } impl GenericQuestion { - pub fn new(id: u32, class: String, text: String, options: Vec, default_option: String, data: HashMap) -> Self { + pub fn new( + id: u32, + class: String, + text: String, + options: Vec, + default_option: String, + data: HashMap, + ) -> Self { info!("Creating new generic question with text {}", text); Self { id, From a893de2a7a9718459d498ee0588c0d82dae1c712 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 07:25:14 +0200 Subject: [PATCH 20/41] remove additional logging --- rust/agama-dbus-server/src/questions.rs | 2 -- service/lib/agama/storage/callbacks/activate_luks.rb | 2 -- web/src/client/questions.js | 6 ------ .../components/questions/LuksActivationQuestion.jsx | 10 ---------- web/src/components/questions/Questions.jsx | 2 -- 5 files changed, 22 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index c7809379bd..9c57ea8b1d 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -65,13 +65,11 @@ struct WithPasswordObject(questions::WithPassword); impl WithPasswordObject { #[dbus_interface(property)] pub fn password(&self) -> &str { - info!("Reading password {}", self.0.password.as_str()); self.0.password.as_str() } #[dbus_interface(property)] pub fn set_password(&mut self, value: &str) -> () { - info!("Setting password to {}", value); self.0.password = value.to_string(); } } diff --git a/service/lib/agama/storage/callbacks/activate_luks.rb b/service/lib/agama/storage/callbacks/activate_luks.rb index b8c07157fa..d13d42178a 100644 --- a/service/lib/agama/storage/callbacks/activate_luks.rb +++ b/service/lib/agama/storage/callbacks/activate_luks.rb @@ -50,8 +50,6 @@ def call(info, attempt) question = question(info, attempt) questions_client.ask(question) do |question_client| - # TODO: remove! - @logger.info("answer: #{question_client.answer.inspect} pwd #{question_client.password.inspect}") activate = question_client.answer == :decrypt password = question_client.password diff --git a/web/src/client/questions.js b/web/src/client/questions.js index e0fb9daeda..db9083c323 100644 --- a/web/src/client/questions.js +++ b/web/src/client/questions.js @@ -84,7 +84,6 @@ function buildQuestion(dbusQuestion) { const ifacesAndProperties = getIfacesAndProperties(dbusQuestion); if (ifaces.includes(DBUS_CONFIG.question.ifaces.generic)) { - console.log("adding generic question"); const dbusProperties = ifacesAndProperties[DBUS_CONFIG.question.ifaces.generic]; question.type = QUESTION_TYPES.generic; @@ -98,7 +97,6 @@ function buildQuestion(dbusQuestion) { } if (ifaces.includes(DBUS_CONFIG.question.ifaces.withPassword)) { - console.log("adding withPassword interface"); const dbusProperties = ifacesAndProperties[DBUS_CONFIG.question.ifaces.withPassword]; question.type = QUESTION_TYPES.withPassword; @@ -144,13 +142,9 @@ class QuestionsClient { * @param {Object} question */ async answer(question) { - console.log("answering question"); const path = DBUS_CONFIG.questions.path + "/" + question.id; if (question.type === QUESTION_TYPES.withPassword) { - console.log("answering question with password"); - // TODO: REMOVE - console.log(question); const proxy = await this.client.proxy(DBUS_CONFIG.question.ifaces.withPassword, path); proxy.Password = question.password; } diff --git a/web/src/components/questions/LuksActivationQuestion.jsx b/web/src/components/questions/LuksActivationQuestion.jsx index 7123eecaec..51b72bc2b0 100644 --- a/web/src/components/questions/LuksActivationQuestion.jsx +++ b/web/src/components/questions/LuksActivationQuestion.jsx @@ -35,26 +35,16 @@ const renderAlert = (attempt) => { export default function LuksActivationQuestion({ question, answerCallback }) { const [password, setPassword] = useState(question.password || ""); - console.log("password value"); - console.log(password); const conditions = { disable: { decrypt: password === "" } }; const defaultAction = "decrypt"; - console.log("luks question:"); - console.log(question); - const actionCallback = (option) => { - console.log("answering question"); question.password = password; question.answer = option; - console.log(question); answerCallback(question); }; const triggerDefaultAction = (e) => { - console.log("form action"); - console.log("password value"); - console.log(password); e.preventDefault(); if (!conditions.disable?.[defaultAction]) { actionCallback(defaultAction); diff --git a/web/src/components/questions/Questions.jsx b/web/src/components/questions/Questions.jsx index 8c1fde281b..0912fc8aef 100644 --- a/web/src/components/questions/Questions.jsx +++ b/web/src/components/questions/Questions.jsx @@ -63,12 +63,10 @@ export default function Questions() { // Renders the first pending question const [currentQuestion] = pendingQuestions; let QuestionComponent = GenericQuestion; - console.log(currentQuestion); // show specialized popup for luks activation question // more can follow as it will be needed if (currentQuestion.class === "storage.luks_activation") { QuestionComponent = LuksActivationQuestion; - console.log("get luks question"); } return ; } From e3ebfa8d90c02cbf73edcb201168fa97c83775a3 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 09:19:06 +0200 Subject: [PATCH 21/41] adapt js tests --- web/src/client/questions.test.js | 33 +++++++++++-------- .../questions/LuksActivationQuestion.test.jsx | 12 +++++-- .../components/questions/Questions.test.jsx | 2 +- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/web/src/client/questions.test.js b/web/src/client/questions.test.js index 5c800734f5..00aee2187a 100644 --- a/web/src/client/questions.test.js +++ b/web/src/client/questions.test.js @@ -26,14 +26,14 @@ jest.mock("./dbus"); // NOTE: should we export them? const GENERIC_IFACE = "org.opensuse.Agama.Questions1.Generic"; -const LUKS_ACTIVATION_IFACE = "org.opensuse.Agama.Questions1.LuksActivation"; +const WITH_PASSWORD_IFACE = "org.opensuse.Agama.Questions1.WithPassword"; const questionProxy = { wait: jest.fn(), Answer: "" }; -const luksActivationProxy = { +const withPasswordProxy = { wait: jest.fn(), Password: "" }; @@ -46,6 +46,10 @@ const ifacesAndProperties = { t: "u", v: 432 }, + Class: { + t: "s", + v: "storage.luks_activation" + }, Text: { t: "s", v: "The device /dev/vdb1 (2.00 GiB) is encrypted. Do you want to decrypt it?" @@ -59,22 +63,22 @@ const ifacesAndProperties = { }, DefaultOption: { t: "s", - v: "" + v: "decrypt" + }, + Data: { + t: "a{ss}", + v: { Attempt: "1" } }, Answer: { t: "s", v: "" } }, - "org.opensuse.Agama.Questions1.LuksActivation": { + "org.opensuse.Agama.Questions1.WithPassword": { Password: { t: "s", v: "" }, - Attempt: { - t: "u", - v: 1 - } } }; @@ -85,19 +89,20 @@ const getManagedObjectsMock = [ const expectedQuestions = [ { id: 432, - type: "luksActivation", + class: "storage.luks_activation", + type: "withPassword", text: "The device /dev/vdb1 (2.00 GiB) is encrypted. Do you want to decrypt it?", options: ["skip", "decrypt"], - defaultOption: "", + defaultOption: "decrypt", answer: "", - attempt: 1, + data: { Attempt: "1" }, password: "", } ]; const proxies = { [GENERIC_IFACE]: questionProxy, - [LUKS_ACTIVATION_IFACE]: luksActivationProxy + [WITH_PASSWORD_IFACE]: withPasswordProxy }; beforeEach(() => { @@ -133,14 +138,14 @@ describe("#answer", () => { describe("when answering a question implementing the LUKS activation interface", () => { beforeEach(() => { - question = { id: 432, type: 'luksActivation', answer: 'skip', password: 'notSecret' }; + question = { id: 432, type: 'withPassword', class: 'storage.luks_activation', answer: 'skip', password: 'notSecret' }; }); it("sets given password", async () => { const client = new QuestionsClient(); await client.answer(question); - expect(luksActivationProxy).toMatchObject({ Password: "notSecret" }); + expect(withPasswordProxy).toMatchObject({ Password: "notSecret" }); }); }); }); diff --git a/web/src/components/questions/LuksActivationQuestion.test.jsx b/web/src/components/questions/LuksActivationQuestion.test.jsx index fd9c53cd53..cffffcde1f 100644 --- a/web/src/components/questions/LuksActivationQuestion.test.jsx +++ b/web/src/components/questions/LuksActivationQuestion.test.jsx @@ -35,9 +35,11 @@ describe("LuksActivationQuestion", () => { beforeEach(() => { question = { id: 1, + class: "storage.luks_activation", text: "A Luks device found. Do you want to open it?", - attempt: 1, options: ["decrypt", "skip"], + defaultOption: "decrypt", + data: { attempt: "1" }, }; }); @@ -67,9 +69,11 @@ describe("LuksActivationQuestion", () => { beforeEach(() => { question = { id: 1, + class: "storage.luks_activation", text: "A Luks device found. Do you want to open it?", - attempt: 3, options: ["decrypt", "skip"], + defaultOption: "decrypt", + data: { attempt: "3" }, }; }); @@ -84,9 +88,11 @@ describe("LuksActivationQuestion", () => { beforeEach(() => { question = { id: 1, + class: "storage.luks_activation", text: "A Luks device found. Do you want to open it?", - attempt: 1, options: ["decrypt", "skip"], + defaultOption: "decrypt", + data: { attempt: "1" }, }; }); diff --git a/web/src/components/questions/Questions.test.jsx b/web/src/components/questions/Questions.test.jsx index e79a52c053..de92ff53cb 100644 --- a/web/src/components/questions/Questions.test.jsx +++ b/web/src/components/questions/Questions.test.jsx @@ -33,7 +33,7 @@ jest.mock("~/components/questions/LuksActivationQuestion", () => mockComponent(" const handlers = {}; const genericQuestion = { id: 1, type: 'generic' }; -const luksActivationQuestion = { id: 1, type: 'luksActivation' }; +const luksActivationQuestion = { id: 1, class: "storage.luks_activation" }; let pendingQuestions = []; beforeEach(() => { From ab510b45b6a3ab3829feed403e801542cc1d5dd2 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 09:37:34 +0200 Subject: [PATCH 22/41] remove test for luks as class is now almost empty --- .../agama/luks_activation_question_test.rb | 53 ------------------- 1 file changed, 53 deletions(-) delete mode 100644 service/test/agama/luks_activation_question_test.rb diff --git a/service/test/agama/luks_activation_question_test.rb b/service/test/agama/luks_activation_question_test.rb deleted file mode 100644 index 0db079523d..0000000000 --- a/service/test/agama/luks_activation_question_test.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require_relative "../test_helper" -require "agama/luks_activation_question" - -describe Agama::LuksActivationQuestion do - describe ".new" do - it "creates a question with the text to ask for a LUKS activation" do - question = described_class.new("/dev/sda1") - expect(question.text).to match(/device \/dev\/sda1 is encrypted/) - - question = described_class.new("/dev/sda1", label: "mydata") - expect(question.text).to match(/device \/dev\/sda1 mydata is encrypted/) - - question = described_class.new("/dev/sda1", size: "5 GiB") - expect(question.text).to match(/device \/dev\/sda1 \(5 GiB\) is encrypted/) - - question = described_class.new("/dev/sda1", label: "mydata", size: "5 GiB") - expect(question.text).to match(/device \/dev\/sda1 mydata \(5 GiB\) is encrypted/) - end - end - - subject { described_class.new("/dev/sda1") } - - it "has no default option" do - expect(subject.default_option).to be_nil - end - - describe "#options" do - it "returns :skip and :decrypt" do - expect(subject.options).to contain_exactly(:skip, :decrypt) - end - end -end From 65d2733f1b028514e14144853f3f3fed46ef5555 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 16:03:12 +0200 Subject: [PATCH 23/41] adapt another ruby test --- service/test/agama/storage/callbacks/activate_luks_test.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/service/test/agama/storage/callbacks/activate_luks_test.rb b/service/test/agama/storage/callbacks/activate_luks_test.rb index 323d8175e6..b0a2f681cb 100644 --- a/service/test/agama/storage/callbacks/activate_luks_test.rb +++ b/service/test/agama/storage/callbacks/activate_luks_test.rb @@ -21,7 +21,7 @@ require_relative "../../../test_helper" require "agama/storage/callbacks/activate_luks" -require "agama/luks_activation_question" +require "agama/question_with_password" require "agama/dbus/clients/questions" require "agama/dbus/clients/question" require "storage" @@ -51,7 +51,7 @@ it "asks a question to activate a LUKS device" do expect(questions_client).to receive(:ask) do |question| - expect(question).to be_a(Agama::LuksActivationQuestion) + expect(question).to be_a(Agama::QuestionWithPassword) end subject.call(luks_info, attempt) From 0a27a51d1763a49a58eca7c54ee3a9526817a0ac Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 16:21:55 +0200 Subject: [PATCH 24/41] remove test for question which become dummy data holder --- service/test/agama/question_test.rb | 81 ----------------------------- 1 file changed, 81 deletions(-) delete mode 100644 service/test/agama/question_test.rb diff --git a/service/test/agama/question_test.rb b/service/test/agama/question_test.rb deleted file mode 100644 index 00e121be17..0000000000 --- a/service/test/agama/question_test.rb +++ /dev/null @@ -1,81 +0,0 @@ -# frozen_string_literal: true - -# Copyright (c) [2022] SUSE LLC -# -# All Rights Reserved. -# -# This program is free software; you can redistribute it and/or modify it -# under the terms of version 2 of the GNU General Public License as published -# by the Free Software Foundation. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or -# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for -# more details. -# -# You should have received a copy of the GNU General Public License along -# with this program; if not, contact SUSE LLC. -# -# To contact SUSE LLC about this file by physical or electronic mail, you may -# find current contact information at www.suse.com. - -require_relative "../test_helper" -require "agama/question" - -describe Agama::Question do - describe ".new" do - it "creates a question with unique id" do - question1 = described_class.new("test1") - question2 = described_class.new("test2") - question3 = described_class.new("test3") - - ids = [question1, question2, question3].map(&:id).uniq - - expect(ids.size).to eq(3) - end - end - - subject { described_class.new("test", options: options, default_option: default_option) } - - let(:options) { [:yes, :no] } - - let(:default_option) { :yes } - - describe "#answer=" do - context "when the given value is a valid option" do - let(:value) { :no } - - it "sets the given option as answer" do - subject.answer = value - - expect(subject.answer).to eq(value) - end - end - - context "when the given value is not a valid option" do - let(:value) { :other } - - it "raises an error" do - expect { subject.answer = value }.to raise_error(ArgumentError, /Invalid answer/) - end - end - end - - describe "#answered?" do - context "if the question has no answer yet" do - it "returns false" do - expect(subject.answered?).to eq(false) - end - end - - context "if the question has an answer" do - before do - subject.answer = :yes - end - - it "returns true" do - expect(subject.answered?).to eq(true) - end - end - end -end From eda57afe660646e41ff59cc4950c6f0e4c796f22 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 16:26:03 +0200 Subject: [PATCH 25/41] move adaptation for new iface --- service/test/agama/dbus/clients/question_test.rb | 14 +++++++------- service/test/agama/dbus/clients/questions_test.rb | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/service/test/agama/dbus/clients/question_test.rb b/service/test/agama/dbus/clients/question_test.rb index 3050045fdc..aa9ccfed18 100644 --- a/service/test/agama/dbus/clients/question_test.rb +++ b/service/test/agama/dbus/clients/question_test.rb @@ -31,17 +31,17 @@ .and_return(dbus_object) allow(dbus_object).to receive(:[]).with("org.opensuse.Agama.Questions1.Generic") .and_return(generic_iface) - allow(dbus_object).to receive(:[]).with("org.opensuse.Agama.Questions1.LuksActivation") - .and_return(luks_iface) - allow(dbus_object).to receive(:has_iface?).with(/LuksActivation/).and_return(luks_iface?) + allow(dbus_object).to receive(:[]).with("org.opensuse.Agama.Questions1.WithPassword") + .and_return(with_password_iface) + allow(dbus_object).to receive(:has_iface?).with(/WithPassword/).and_return(with_password?) end let(:bus) { instance_double(Agama::DBus::Bus) } let(:service) { instance_double(::DBus::ProxyService) } let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:generic_iface) { instance_double(::DBus::ProxyObjectInterface) } - let(:luks_iface) { instance_double(::DBus::ProxyObjectInterface) } - let(:luks_iface?) { true } + let(:with_password_iface) { instance_double(::DBus::ProxyObjectInterface) } + let(:with_password?) { true } subject { described_class.new("/org/opensuse/Agama/Questions1/23") } @@ -61,12 +61,12 @@ describe "#password" do it "returns the appropriate property of the luks interface" do - expect(luks_iface).to receive(:[]).with("Password").and_return("the password") + expect(with_password_iface).to receive(:[]).with("Password").and_return("the password") expect(subject.password).to eq "the password" end context "when the luks interface is missing" do - let(:luks_iface?) { false } + let(:with_password?) { false } it "returns nil" do expect(subject.password).to be_nil diff --git a/service/test/agama/dbus/clients/questions_test.rb b/service/test/agama/dbus/clients/questions_test.rb index b4b23bb518..1bfa5241cf 100644 --- a/service/test/agama/dbus/clients/questions_test.rb +++ b/service/test/agama/dbus/clients/questions_test.rb @@ -44,7 +44,7 @@ let(:question1) { Agama::Question.new("What?", options: [:this, :that]) } let(:question2) do - Agama::Question.new("When?", options: [:now, :later], default_option: :now) + Agama::Question.new(text: "When?", qclass: "test", options: [:now, :later], default_option: :now) end let(:question1_proxy) do instance_double(::DBus::ProxyObject, path: "/org/opensuse/Agama/Questions1/33") From 36a77239cf16907d16a9ae19b83f7d1e0cf3da8a Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 16:38:18 +0200 Subject: [PATCH 26/41] more test adaptation and rubocop fixes --- service/test/agama/dbus/clients/questions_test.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/service/test/agama/dbus/clients/questions_test.rb b/service/test/agama/dbus/clients/questions_test.rb index 1bfa5241cf..3f97d8aee9 100644 --- a/service/test/agama/dbus/clients/questions_test.rb +++ b/service/test/agama/dbus/clients/questions_test.rb @@ -42,9 +42,13 @@ let(:dbus_object) { instance_double(::DBus::ProxyObject) } let(:properties_iface) { instance_double(::DBus::ProxyObjectInterface) } - let(:question1) { Agama::Question.new("What?", options: [:this, :that]) } + let(:question1) do + Agama::Question.new(text: "What?", qclass: "test2", options: [:this, :that], + default_option: :this) + end let(:question2) do - Agama::Question.new(text: "When?", qclass: "test", options: [:now, :later], default_option: :now) + Agama::Question.new(text: "When?", qclass: "test", options: [:now, :later], + default_option: :now) end let(:question1_proxy) do instance_double(::DBus::ProxyObject, path: "/org/opensuse/Agama/Questions1/33") From 74462a0f9fc04f6ba247f584b1e39ba72442a653 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Thu, 13 Jul 2023 16:44:22 +0200 Subject: [PATCH 27/41] more test adaptation and rubocop fixes --- service/test/agama/dbus/clients/questions_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/service/test/agama/dbus/clients/questions_test.rb b/service/test/agama/dbus/clients/questions_test.rb index 3f97d8aee9..2efec2c8eb 100644 --- a/service/test/agama/dbus/clients/questions_test.rb +++ b/service/test/agama/dbus/clients/questions_test.rb @@ -62,7 +62,7 @@ let(:dbus_object) { double(::DBus::ProxyObject) } it "asks the service to add a question and returns a stub object for it" do - expect(dbus_object).to receive(:New).with("What?", ["this", "that"], []) + expect(dbus_object).to receive(:New).with("test2", "What?", ["this", "that"], "this", {}) expect(Agama::DBus::Clients::Question).to receive(:new).and_return(question1_stub) expect(subject.add(question1)).to eq question1_stub end From 65d15478bf0fed16429937e4e00e68d5695b6027 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Fri, 14 Jul 2023 11:10:37 +0200 Subject: [PATCH 28/41] document questions dbus interface --- .../org.opensuse.Agama.Questions1.doc.xml | 67 ++++++++++++++++--- 1 file changed, 57 insertions(+), 10 deletions(-) diff --git a/doc/dbus/org.opensuse.Agama.Questions1.doc.xml b/doc/dbus/org.opensuse.Agama.Questions1.doc.xml index 16d4bb24ca..9cf5fb0b4f 100644 --- a/doc/dbus/org.opensuse.Agama.Questions1.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Questions1.doc.xml @@ -17,26 +17,73 @@ dynamically exported in a tree under the */org/opensuse/Agama/Questions1* path, Each D-Bus question implements its own set of interfaces, depending on the type of question. For example, a generic question implements *org.opensuse.Agama.Question1*. And a question asking -for the activation of a LUKS device also implements *org.opensuse.Agama.Questions1.LuksActivation*. +for the activation of a LUKS device also implements *org.opensuse.Agama.Questions1.WithPassword*. Questions can be "unexported" from the ObjectManager tree. The service typically unexports a question -when the question is answered. +when the question is answered and the answer is successfully read. --> + + - - + + + - - - - - - + + + + + + + + + + From 31cd63a791b344914d9491b1efce054db102a982 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 14 Jul 2023 14:54:34 +0200 Subject: [PATCH 29/41] doc/dbus: sort by interface when seeding Rust zbus unfortunately outputs the provided interfaces in a random order in the introspection data, so we have to sort them before comparing --- doc/dbus/bus/cleanup-zbus.xslt | 28 ++++++++++++++++++++++++++++ doc/dbus/bus/seed.sh | 21 +++++++++++++++++++-- 2 files changed, 47 insertions(+), 2 deletions(-) create mode 100644 doc/dbus/bus/cleanup-zbus.xslt diff --git a/doc/dbus/bus/cleanup-zbus.xslt b/doc/dbus/bus/cleanup-zbus.xslt new file mode 100644 index 0000000000..7551056dab --- /dev/null +++ b/doc/dbus/bus/cleanup-zbus.xslt @@ -0,0 +1,28 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/doc/dbus/bus/seed.sh b/doc/dbus/bus/seed.sh index 51f6885ce5..194cd4b867 100755 --- a/doc/dbus/bus/seed.sh +++ b/doc/dbus/bus/seed.sh @@ -3,15 +3,31 @@ abusctl() { busctl --address=unix:path=/run/agama/bus "$@" } +# a stdio filter for XML introspection, +# to fix some less clever choices made by zbus: +# - remove detailed introspection of _child_ nodes +# - make interfaces order deterministic by sorting them +cleanup() { + # also remove the DTD declaration + # otherwise xmlstarlet will complain about it not being available + sed -e '/^ ${DD}1.Manager.bus.xml look() { abusctl tree --list $DD.${1%.*} abusctl introspect --xml-interface $DD.${1%.*} $SS/${1//./\/} \ + | cleanup \ > $DD.$1.bus.xml } @@ -25,9 +41,8 @@ look Users1 abusctl introspect --xml-interface \ ${DD}.Questions1 \ ${SS}/Questions1 \ - | sed -e '/ ${DD}.Questions1.bus.xml -# ^ remove detailed introspection of subnodes abusctl call \ ${DD}.Questions1 \ @@ -37,6 +52,7 @@ abusctl call \ abusctl introspect --xml-interface \ ${DD}.Questions1 \ ${SS}/Questions1/0 \ + | cleanup \ > ${DD}.Questions1.Generic.bus.xml abusctl call \ @@ -47,4 +63,5 @@ abusctl call \ abusctl introspect --xml-interface \ ${DD}.Questions1 \ ${SS}/Questions1/1 \ + | cleanup \ > ${DD}.Questions1.LuksActivation.bus.xml From 0438dbfc849c304117a19f54397b10e3f43f5318 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 14 Jul 2023 15:00:25 +0200 Subject: [PATCH 30/41] setup.sh: exit as soon as a step fails setup-service in particular --- setup.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/setup.sh b/setup.sh index cdf0f51e2a..2f4bcaeb24 100755 --- a/setup.sh +++ b/setup.sh @@ -3,6 +3,9 @@ # This script sets up the development environment without installing any # package. This script is supposed to run within a repository clone. +# Exit on error; unset variables are an error +set -eu + MYDIR=$(realpath $(dirname $0)) # Helper: @@ -25,7 +28,7 @@ $MYDIR/setup-service.sh # Install Frontend dependencies $SUDO zypper --non-interactive --gpg-auto-import-keys install \ - make git 'npm>=18' cockpit || exit 1 + make git 'npm>=18' cockpit # Web Frontend From 594e21eba71def411e7a0a238177ab7b224c12da Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 14 Jul 2023 15:09:45 +0200 Subject: [PATCH 31/41] doc/dbus/bus: adjusted the introspected files after seed.sh/cleanup() they are semantically the same, just with interfaces sorted by name --- .../bus/org.opensuse.Agama.Locale1.bus.xml | 29 ++++---- ....opensuse.Agama.Questions1.Generic.bus.xml | 29 ++++---- ...se.Agama.Questions1.LuksActivation.bus.xml | 19 ++--- .../bus/org.opensuse.Agama.Questions1.bus.xml | 73 +++++++++---------- ....opensuse.Agama.Software1.Proposal.bus.xml | 13 ++-- .../bus/org.opensuse.Agama.Software1.bus.xml | 41 +++++------ .../bus/org.opensuse.Agama.Users1.bus.xml | 29 ++++---- .../bus/org.opensuse.Agama1.Manager.bus.xml | 31 ++++---- 8 files changed, 124 insertions(+), 140 deletions(-) diff --git a/doc/dbus/bus/org.opensuse.Agama.Locale1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Locale1.bus.xml index b8a0dfb0c7..6e40291197 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Locale1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Locale1.bus.xml @@ -1,7 +1,17 @@ - - + + + + + + + + + + + + + @@ -48,17 +58,4 @@ - - - - - - - - - - - - - diff --git a/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml index 44e8169cb6..9ddda86523 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml @@ -1,7 +1,17 @@ - - + + + + + + + + + + + + + @@ -26,13 +36,6 @@ - - - - - - - @@ -40,10 +43,4 @@ - - - - - - diff --git a/doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml index 4443ec2189..6ae24aaac9 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml @@ -1,6 +1,4 @@ - - + @@ -14,13 +12,6 @@ - - - - - - - @@ -45,9 +36,15 @@ + + + + + + + - diff --git a/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml index 4f27c734c5..793ac1d0f3 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml @@ -1,41 +1,10 @@ - - + - - - - + + - - - - - - - - - - - - - - - - - - - - - @@ -58,6 +27,13 @@ + + + + + + + @@ -82,10 +58,31 @@ - - - + + + + + + + + + + + + + + + + + + + - diff --git a/doc/dbus/bus/org.opensuse.Agama.Software1.Proposal.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Software1.Proposal.bus.xml index ae0a38b859..b501f0d052 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Software1.Proposal.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Software1.Proposal.bus.xml @@ -1,6 +1,10 @@ - + + + + + + @@ -22,11 +26,6 @@ - - - - - diff --git a/doc/dbus/bus/org.opensuse.Agama.Software1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Software1.bus.xml index ae45ba2037..9e5a062653 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Software1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Software1.bus.xml @@ -1,7 +1,11 @@ - + - + + + + + + @@ -23,24 +27,6 @@ - - - - - - - - - - - - - - - - - - @@ -71,4 +57,17 @@ + + + + + + + + + + + + + diff --git a/doc/dbus/bus/org.opensuse.Agama.Users1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Users1.bus.xml index ade59a4671..1d0b0d89de 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Users1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Users1.bus.xml @@ -1,6 +1,10 @@ - + + + + + + @@ -22,19 +26,6 @@ - - - - - - - - - - - - - @@ -66,4 +57,12 @@ + + + + + + + + diff --git a/doc/dbus/bus/org.opensuse.Agama1.Manager.bus.xml b/doc/dbus/bus/org.opensuse.Agama1.Manager.bus.xml index ff8365b815..7127266bdc 100644 --- a/doc/dbus/bus/org.opensuse.Agama1.Manager.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama1.Manager.bus.xml @@ -1,6 +1,10 @@ - + + + + + + @@ -22,20 +26,6 @@ - - - - - - - - - - - - - - @@ -55,4 +45,13 @@ + + + + + + + + + From f95e44b88461e3d1171a869389ba38cc53b9c3fa Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 14 Jul 2023 15:16:28 +0200 Subject: [PATCH 32/41] doc/dbus/bus: Storage1: adjusted the introspected files after seed.sh/cleanup() had to manually re-add the interfaces missing on x86: --- .../bus/org.opensuse.Agama.Storage1.bus.xml | 109 +++++++++--------- 1 file changed, 54 insertions(+), 55 deletions(-) diff --git a/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml index 5648db817b..b1f256e401 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Storage1.bus.xml @@ -1,8 +1,25 @@ - + + + + + + + + + + + + + + + + + + + @@ -24,36 +41,6 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -63,29 +50,6 @@ - - - - - - - - - - - - - - - - - - - - - - - @@ -108,9 +72,44 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + From 721c5eec7fb964cc926019b10b19ffd1e45af137 Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 14 Jul 2023 16:00:35 +0200 Subject: [PATCH 33/41] doc/dbus/bus: update seeded Questions API --- ....opensuse.Agama.Questions1.Generic.bus.xml | 2 ++ ...use.Agama.Questions1.WithPassword.bus.xml} | 5 +++-- .../bus/org.opensuse.Agama.Questions1.bus.xml | 20 +++++++++++++------ doc/dbus/bus/seed.sh | 6 +++--- 4 files changed, 22 insertions(+), 11 deletions(-) rename doc/dbus/bus/{org.opensuse.Agama.Questions1.LuksActivation.bus.xml => org.opensuse.Agama.Questions1.WithPassword.bus.xml} (90%) diff --git a/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml index 9ddda86523..60e47935aa 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Questions1.Generic.bus.xml @@ -38,6 +38,8 @@ + + diff --git a/doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Questions1.WithPassword.bus.xml similarity index 90% rename from doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml rename to doc/dbus/bus/org.opensuse.Agama.Questions1.WithPassword.bus.xml index 6ae24aaac9..eaf99e6392 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Questions1.LuksActivation.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Questions1.WithPassword.bus.xml @@ -38,13 +38,14 @@ + + - - + diff --git a/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml b/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml index 793ac1d0f3..c9d9c8a126 100644 --- a/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml +++ b/doc/dbus/bus/org.opensuse.Agama.Questions1.bus.xml @@ -63,19 +63,22 @@ creates new generic question without answer --> + - + + - - - - - + + + + + + + + diff --git a/doc/dbus/bus/seed.sh b/doc/dbus/bus/seed.sh index 194cd4b867..4284a4604c 100755 --- a/doc/dbus/bus/seed.sh +++ b/doc/dbus/bus/seed.sh @@ -48,7 +48,7 @@ abusctl call \ ${DD}.Questions1 \ ${SS}/Questions1 \ ${DD}.Questions1 \ - New sasas "should I stay or should I go" 2 yes no 1 yes + New "ssassa{ss}" "org.bands.Clash" "should I stay or should I go" 2 yes no yes 0 abusctl introspect --xml-interface \ ${DD}.Questions1 \ ${SS}/Questions1/0 \ @@ -59,9 +59,9 @@ abusctl call \ ${DD}.Questions1 \ ${SS}/Questions1 \ ${DD}.Questions1 \ - NewLuksActivation sssy "/dev/tape1" "ZX Spectrum games" "90 minutes" 1 + NewWithPassword "ssassa{ss}" "world.MiddleEarth.Moria.gate1" "Speak friend and enter" 2 enter giveup giveup 0 abusctl introspect --xml-interface \ ${DD}.Questions1 \ ${SS}/Questions1/1 \ | cleanup \ - > ${DD}.Questions1.LuksActivation.bus.xml + > ${DD}.Questions1.WithPassword.bus.xml From f6429d7103fc1a30a04a94d6058fe833e74c7f3f Mon Sep 17 00:00:00 2001 From: Martin Vidner Date: Fri, 14 Jul 2023 16:12:17 +0200 Subject: [PATCH 34/41] doc/dbus: Questions1.*.doc.xml now pass make check The @result output argument is unfortunately unnamed in zbus(Rust) introspection so we have to document it in plain text --- ....opensuse.Agama.Questions1.Generic.doc.xml | 16 ++++++++++++++++ ...se.Agama.Questions1.LuksActivation.doc.xml | 19 ------------------- ...suse.Agama.Questions1.WithPassword.doc.xml | 12 ++++++++++++ .../org.opensuse.Agama.Questions1.doc.xml | 18 +++++++++++------- 4 files changed, 39 insertions(+), 26 deletions(-) delete mode 100644 doc/dbus/org.opensuse.Agama.Questions1.LuksActivation.doc.xml create mode 100644 doc/dbus/org.opensuse.Agama.Questions1.WithPassword.doc.xml diff --git a/doc/dbus/org.opensuse.Agama.Questions1.Generic.doc.xml b/doc/dbus/org.opensuse.Agama.Questions1.Generic.doc.xml index 8d6f667341..f3aba26a59 100644 --- a/doc/dbus/org.opensuse.Agama.Questions1.Generic.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Questions1.Generic.doc.xml @@ -8,6 +8,22 @@ --> + + + + + + - - - - - - - diff --git a/doc/dbus/org.opensuse.Agama.Questions1.WithPassword.doc.xml b/doc/dbus/org.opensuse.Agama.Questions1.WithPassword.doc.xml new file mode 100644 index 0000000000..ee8f841032 --- /dev/null +++ b/doc/dbus/org.opensuse.Agama.Questions1.WithPassword.doc.xml @@ -0,0 +1,12 @@ + + + + + + + + diff --git a/doc/dbus/org.opensuse.Agama.Questions1.doc.xml b/doc/dbus/org.opensuse.Agama.Questions1.doc.xml index 9cf5fb0b4f..eddea73a4a 100644 --- a/doc/dbus/org.opensuse.Agama.Questions1.doc.xml +++ b/doc/dbus/org.opensuse.Agama.Questions1.doc.xml @@ -36,9 +36,10 @@ when the question is answered and the answer is successfully read. @data: additional data that is specific to given question class. Can be used for additional details about question that helps with UI or with matching automatic answers. - @result: Object path of created question. - Creates new generic question + Creates new generic question. + Returns the object path of the created question. + --> @@ -46,10 +47,11 @@ when the question is answered and the answer is successfully read. - + + @@ -72,11 +74,12 @@ when the question is answered and the answer is successfully read. - + + - + + From 960024dffb0474658eb98e5d5bcbfe8573ce06b6 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 17 Jul 2023 10:54:19 +0200 Subject: [PATCH 35/41] improve documentation --- rust/agama-dbus-server/src/questions.rs | 32 +++++++++++++++++++++++-- rust/agama-lib/src/questions.rs | 19 +++++++++++---- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index e8f7404f90..e9acab9e18 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -6,7 +6,7 @@ use agama_lib::{ questions::{self, GenericQuestion, WithPassword}, }; use anyhow::Context; -use log::{self, info}; +use log; use zbus::{dbus_interface, fdo::ObjectManager, zvariant::ObjectPath, Connection}; #[derive(Clone, Debug)] @@ -80,13 +80,27 @@ enum QuestionType { BaseWithPassword, } +/// Trait for objects that can provide answers to all kind of Question. trait AnswerStrategy { /// Provides answer for generic question + /// + /// I gets as argument the question to answer. Returned value is `answer` + /// property or None. If `None` is used, it means that this object does not + /// answer to given question. fn answer(&self, question: &GenericQuestion) -> Option; /// Provides answer and password for base question with password + /// + /// I gets as argument the question to answer. Returned value is pair + /// of `answer` and `password` properties. If `None` is used in any + /// position it means that this object does not respond to given property. + /// + /// It is object responsibility to provide correct pair. For example if + /// possible answer can be "Ok" and "Cancel". Then for `Ok` password value + /// should be provided and for `Cancel` it can be `None`. fn answer_with_password(&self, question: &WithPassword) -> (Option, Option); } +/// AnswerStrategy that provides as answer the default option. struct DefaultAnswers; impl AnswerStrategy for DefaultAnswers { @@ -118,6 +132,7 @@ impl Questions { default_option: &str, data: HashMap, ) -> Result { + log::info!("Creating new question with text: {}.", text); let id = self.last_id; self.last_id += 1; // TODO use some thread safety let options = options.iter().map(|o| o.to_string()).collect(); @@ -150,6 +165,7 @@ impl Questions { default_option: &str, data: HashMap, ) -> Result { + log::info!("Creating new question with password with text: {}.", text); let id = self.last_id; self.last_id += 1; // TODO use some thread safety // TODO: share code better @@ -232,16 +248,27 @@ impl Questions { } /// tries to provide answer to question using answer strategies + /// + /// What happens underhood is that it user answer_strategies vector + /// and try to find the first strategy that provides answer. When + /// aswer is provided, it returns immediatelly. fn fill_answer(&self, question: &mut GenericQuestion) -> () { for strategy in self.answer_strategies.iter() { match strategy.answer(question) { None => (), - Some(answer) => question.answer = answer, + Some(answer) => { + question.answer = answer; + return () + } } } } /// tries to provide answer to question using answer strategies + /// + /// What happens underhood is that it user answer_strategies vector + /// and try to find the first strategy that provides answer. When + /// aswer is provided, it returns immediatelly. fn fill_answer_with_password(&self, question: &mut WithPassword) -> () { for strategy in self.answer_strategies.iter() { let (answer, password) = strategy.answer_with_password(question); @@ -250,6 +277,7 @@ impl Questions { } if let Some(answer) = answer { question.base.answer = answer; + return () } } } diff --git a/rust/agama-lib/src/questions.rs b/rust/agama-lib/src/questions.rs index 2737305381..61f9894f22 100644 --- a/rust/agama-lib/src/questions.rs +++ b/rust/agama-lib/src/questions.rs @@ -1,4 +1,3 @@ -use log::info; use std::collections::HashMap; /// module holdings data model for agama questions @@ -33,7 +32,6 @@ impl GenericQuestion { default_option: String, data: HashMap, ) -> Self { - info!("Creating new generic question with text {}", text); Self { id, class, @@ -51,7 +49,21 @@ impl GenericQuestion { } /// Composition for questions which include password. -/// TODO: research a bit how ideally do mixins in rust +/// +/// ## Extension +/// If there is need to provide more mixins, then this structure does not work +/// well as it is hard do various combinations. Idea is when need for more +/// mixins arise to convert it to Question Struct that have optional mixins +/// inside like +/// +/// struct Question { +/// base: GenericQuestion, +/// with_password: Option, +/// another_mixin: Option +/// } +/// +/// This way all handling code can check if given mixin is used and +/// act appropriate. #[derive(Clone, Debug)] pub struct WithPassword { /// Luks password. Empty means no password set. @@ -62,7 +74,6 @@ pub struct WithPassword { impl WithPassword { pub fn new(base: GenericQuestion) -> Self { - info!("Adding to question with password interface."); Self { password: "".to_string(), base, From 071ebd68e571976f1faa56b9f4aa3353f3caa25f Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 17 Jul 2023 11:07:26 +0200 Subject: [PATCH 36/41] fix formatting --- rust/agama-dbus-server/src/questions.rs | 10 +++++----- rust/agama-lib/src/questions.rs | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index e9acab9e18..bda5443265 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -95,7 +95,7 @@ trait AnswerStrategy { /// position it means that this object does not respond to given property. /// /// It is object responsibility to provide correct pair. For example if - /// possible answer can be "Ok" and "Cancel". Then for `Ok` password value + /// possible answer can be "Ok" and "Cancel". Then for `Ok` password value /// should be provided and for `Cancel` it can be `None`. fn answer_with_password(&self, question: &WithPassword) -> (Option, Option); } @@ -248,7 +248,7 @@ impl Questions { } /// tries to provide answer to question using answer strategies - /// + /// /// What happens underhood is that it user answer_strategies vector /// and try to find the first strategy that provides answer. When /// aswer is provided, it returns immediatelly. @@ -258,14 +258,14 @@ impl Questions { None => (), Some(answer) => { question.answer = answer; - return () + return (); } } } } /// tries to provide answer to question using answer strategies - /// + /// /// What happens underhood is that it user answer_strategies vector /// and try to find the first strategy that provides answer. When /// aswer is provided, it returns immediatelly. @@ -277,7 +277,7 @@ impl Questions { } if let Some(answer) = answer { question.base.answer = answer; - return () + return (); } } } diff --git a/rust/agama-lib/src/questions.rs b/rust/agama-lib/src/questions.rs index 61f9894f22..d50c72f07e 100644 --- a/rust/agama-lib/src/questions.rs +++ b/rust/agama-lib/src/questions.rs @@ -55,13 +55,13 @@ impl GenericQuestion { /// well as it is hard do various combinations. Idea is when need for more /// mixins arise to convert it to Question Struct that have optional mixins /// inside like -/// +/// /// struct Question { /// base: GenericQuestion, /// with_password: Option, /// another_mixin: Option /// } -/// +/// /// This way all handling code can check if given mixin is used and /// act appropriate. #[derive(Clone, Debug)] From cd798845861126f3b3fa57a7cdde864ed70506ec Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 17 Jul 2023 11:08:18 +0200 Subject: [PATCH 37/41] apply clippy fixes --- rust/agama-dbus-server/src/questions.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index bda5443265..efb06d4326 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -105,11 +105,11 @@ struct DefaultAnswers; impl AnswerStrategy for DefaultAnswers { fn answer(&self, question: &GenericQuestion) -> Option { - return Some(question.default_option.clone()); + Some(question.default_option.clone()) } fn answer_with_password(&self, question: &WithPassword) -> (Option, Option) { - return (Some(question.base.default_option.clone()), None); + (Some(question.base.default_option.clone()), None) } } @@ -252,13 +252,13 @@ impl Questions { /// What happens underhood is that it user answer_strategies vector /// and try to find the first strategy that provides answer. When /// aswer is provided, it returns immediatelly. - fn fill_answer(&self, question: &mut GenericQuestion) -> () { + fn fill_answer(&self, question: &mut GenericQuestion) { for strategy in self.answer_strategies.iter() { match strategy.answer(question) { None => (), Some(answer) => { question.answer = answer; - return (); + return ; } } } @@ -269,7 +269,7 @@ impl Questions { /// What happens underhood is that it user answer_strategies vector /// and try to find the first strategy that provides answer. When /// aswer is provided, it returns immediatelly. - fn fill_answer_with_password(&self, question: &mut WithPassword) -> () { + fn fill_answer_with_password(&self, question: &mut WithPassword) { for strategy in self.answer_strategies.iter() { let (answer, password) = strategy.answer_with_password(question); if let Some(password) = password { @@ -277,7 +277,7 @@ impl Questions { } if let Some(answer) = answer { question.base.answer = answer; - return (); + return ; } } } From a0f1765cdc2c5e29112c0f127fe82baedc79050f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Imobach=20Gonz=C3=A1lez=20Sosa?= Date: Mon, 17 Jul 2023 10:19:04 +0100 Subject: [PATCH 38/41] Fix typos --- rust/agama-dbus-server/src/questions.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index efb06d4326..92f6ca6c58 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -249,9 +249,9 @@ impl Questions { /// tries to provide answer to question using answer strategies /// - /// What happens underhood is that it user answer_strategies vector + /// What happens under the hood is that it uses answer_strategies vector /// and try to find the first strategy that provides answer. When - /// aswer is provided, it returns immediatelly. + /// answer is provided, it returns immediately. fn fill_answer(&self, question: &mut GenericQuestion) { for strategy in self.answer_strategies.iter() { match strategy.answer(question) { @@ -266,9 +266,9 @@ impl Questions { /// tries to provide answer to question using answer strategies /// - /// What happens underhood is that it user answer_strategies vector + /// What happens under the hood is that it uses answer_strategies vector /// and try to find the first strategy that provides answer. When - /// aswer is provided, it returns immediatelly. + /// answer is provided, it returns immediately. fn fill_answer_with_password(&self, question: &mut WithPassword) { for strategy in self.answer_strategies.iter() { let (answer, password) = strategy.answer_with_password(question); From 54705ac9ebd31d94b947ecc79833fe30a053f748 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 17 Jul 2023 11:21:53 +0200 Subject: [PATCH 39/41] Update changes --- rust/package/agama-cli.changes | 7 +++++++ service/package/rubygem-agama.changes | 7 +++++++ web/package/cockpit-agama.changes | 8 ++++++++ 3 files changed, 22 insertions(+) diff --git a/rust/package/agama-cli.changes b/rust/package/agama-cli.changes index a4e50f5b84..b12f8b9634 100644 --- a/rust/package/agama-cli.changes +++ b/rust/package/agama-cli.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Jul 17 09:16:38 UTC 2023 - Josef Reidinger + +- Adapt to new questions DBus API to allow automatic answering of + questions when requested (gh#openSUSE/agama#637, reverts + gh#openSUSE/agama#649 as now default option is mandatory) + ------------------------------------------------------------------- Thu Jul 13 10:22:36 UTC 2023 - Imobach Gonzalez Sosa diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index fa17a0775c..ce62a27063 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Mon Jul 17 09:16:38 UTC 2023 - Josef Reidinger + +- Adapt to new questions DBus API to allow automatic answering of + questions when requested. All code using this API is adapted + (gh#openSUSE/agama#637) + ------------------------------------------------------------------- Wed Jul 5 14:02:23 UTC 2023 - José Iván López González diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index 75f7c248e6..e8b3ec8bf5 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,3 +1,11 @@ +------------------------------------------------------------------- +Mon Jul 17 09:16:38 UTC 2023 - Josef Reidinger + +- Adapt to new questions DBus API to allow automatic answering of + questions when requested. No visible change in UI, just default + answer for luks partition activation is now Skip. + (gh#openSUSE/agama#637) + ------------------------------------------------------------------- Wed Jul 5 13:59:58 UTC 2023 - José Iván López González From c337aac7668e2976bc668a212304be27e084ecaa Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 17 Jul 2023 11:58:29 +0200 Subject: [PATCH 40/41] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Imobach González Sosa --- rust/package/agama-cli.changes | 2 +- service/package/rubygem-agama.changes | 2 +- web/package/cockpit-agama.changes | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rust/package/agama-cli.changes b/rust/package/agama-cli.changes index b12f8b9634..1d5445bb30 100644 --- a/rust/package/agama-cli.changes +++ b/rust/package/agama-cli.changes @@ -1,7 +1,7 @@ ------------------------------------------------------------------- Mon Jul 17 09:16:38 UTC 2023 - Josef Reidinger -- Adapt to new questions DBus API to allow automatic answering of +- Adapt to new questions D-Bus API to allow automatic answering of questions when requested (gh#openSUSE/agama#637, reverts gh#openSUSE/agama#649 as now default option is mandatory) diff --git a/service/package/rubygem-agama.changes b/service/package/rubygem-agama.changes index ce62a27063..5bdcc703c4 100644 --- a/service/package/rubygem-agama.changes +++ b/service/package/rubygem-agama.changes @@ -1,7 +1,7 @@ ------------------------------------------------------------------- Mon Jul 17 09:16:38 UTC 2023 - Josef Reidinger -- Adapt to new questions DBus API to allow automatic answering of +- Adapt to new questions D-Bus API to allow automatic answering of questions when requested. All code using this API is adapted (gh#openSUSE/agama#637) diff --git a/web/package/cockpit-agama.changes b/web/package/cockpit-agama.changes index e8b3ec8bf5..ce1167cdc9 100644 --- a/web/package/cockpit-agama.changes +++ b/web/package/cockpit-agama.changes @@ -1,9 +1,9 @@ ------------------------------------------------------------------- Mon Jul 17 09:16:38 UTC 2023 - Josef Reidinger -- Adapt to new questions DBus API to allow automatic answering of +- Adapt to new questions D-Bus API to allow automatic answering of questions when requested. No visible change in UI, just default - answer for luks partition activation is now Skip. + answer for LUKS partition activation is now "Skip". (gh#openSUSE/agama#637) ------------------------------------------------------------------- From 765135dfe8fabdaa41092789d99f8fd168ac9da3 Mon Sep 17 00:00:00 2001 From: Josef Reidinger Date: Mon, 17 Jul 2023 11:59:50 +0200 Subject: [PATCH 41/41] fix clippy output --- rust/agama-dbus-server/src/questions.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rust/agama-dbus-server/src/questions.rs b/rust/agama-dbus-server/src/questions.rs index 92f6ca6c58..b84a3a834d 100644 --- a/rust/agama-dbus-server/src/questions.rs +++ b/rust/agama-dbus-server/src/questions.rs @@ -258,7 +258,7 @@ impl Questions { None => (), Some(answer) => { question.answer = answer; - return ; + return; } } } @@ -277,7 +277,7 @@ impl Questions { } if let Some(answer) = answer { question.base.answer = answer; - return ; + return; } } }