From f899326dd481903988b1b947ccff2aca45b3dc27 Mon Sep 17 00:00:00 2001 From: Alexandre Huff Date: Sat, 5 Feb 2022 09:54:32 -0300 Subject: [PATCH] Fix $XAPP_DESCRIPTOR_PATH parser bug While xapp-frame-cpp assumes that $XAPP_DESCRIPTOR_PATH is a filename, the xapp-onboarder sets it up as a directory. This assumption causes xapps based on xapp-frame-cpp to crash on startup even using default helm charts. This change fixes this bug and adds some logic to determine if the $XAPP_DESCRIPTOR_PATH is a directory or a filename. This change also adds test cases for $XAPP_DESCRIPTOR_PATH parser. Issue-ID: RIC-883 Signed-off-by: Alexandre Huff Change-Id: I1d29c8c8ae80028accc5d3e29ea571b6365121ac --- CHANGES | 4 ++++ CMakeLists.txt | 2 +- src/config/config.cpp | 31 ++++++++++++++++++++++++------- test/Makefile | 1 - test/config_test.cpp | 44 +++++++++++++++++++++++--------------------- 5 files changed, 52 insertions(+), 30 deletions(-) diff --git a/CHANGES b/CHANGES index 5cc1d83..5d6ee6e 100644 --- a/CHANGES +++ b/CHANGES @@ -5,6 +5,10 @@ # has several changes. Multiple blank lines between versions are # squished to one. +2022 05 February; version 2.3.6 + Fixed bug on XAPP_DESCRIPTOR_PATH parser. + Add test cases for XAPP_DESCRIPTOR_PATH parser. + release = E 2021 03 December; version 2.3.5 Taking in new RMR version 4.8.0. Also one bug fix added. diff --git a/CMakeLists.txt b/CMakeLists.txt index a06f3f4..b79eb2e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -30,7 +30,7 @@ cmake_minimum_required( VERSION 3.5 ) set( major_version "2" ) # should be automatically populated from git tag later, but until CI process sets a tag we use this set( minor_version "3" ) -set( patch_level "5" ) +set( patch_level "6" ) set( install_root "${CMAKE_INSTALL_PREFIX}" ) set( install_inc "include/ricxfcpp" ) diff --git a/src/config/config.cpp b/src/config/config.cpp index 5259a17..ab44693 100644 --- a/src/config/config.cpp +++ b/src/config/config.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include @@ -111,7 +112,7 @@ void xapp::Config::Listener( ) { if( errno == EAGAIN ) { continue; } else { - fprintf( stderr, " ### CRIT ### config listener read err: %s\n", strerror( errno ) ); return; } } @@ -143,6 +144,10 @@ std::shared_ptr xapp::Config::jparse( std::string ufname ) { fname = ufname; std::ifstream ifs( fname ); + if( ! ifs.is_open() ) { + fprintf( stderr, " ### WARN ### unable to open %s; %s\n", fname.c_str(), strerror( errno ) ); + } + std::string st( (std::istreambuf_iterator( ifs ) ), (std::istreambuf_iterator() ) ); auto new_jh = std::shared_ptr( new xapp::Jhash( st.c_str() ) ); @@ -156,18 +161,30 @@ std::shared_ptr xapp::Config::jparse( std::string ufname ) { The actual meaning of the environment variable is confusing. The name is "path" which should mean that this is the directory in which the config file lives, but the examples - elsewhere suggest that this is a filename (either fully qualified or relative). For now - we will assume that it's a file name, though we could add some intelligence to determine - if it's a directory name or not if it comes to it. + elsewhere suggest that this is a filename (either fully qualified or relative). To prevent + errors, we use some intelligence to determine if it's a directory name or not if it comes to it. */ std::shared_ptr xapp::Config::jparse( ) { const char* data; + std::string filename; + struct stat sb; + + data = getenv( (const char *) "XAPP_DESCRIPTOR_PATH" ); + if( data != NULL ) { + filename = data; + if( stat( data, &sb ) == 0 ) { + if( S_ISDIR( sb.st_mode ) ) { + filename.append( "/config-file.json" ); + } + } else { + fprintf( stderr, " ### ERR ### unable to stat env XAPP_DESCRIPTOR_PATH: %s\n", strerror( errno ) ); + } - if( (data = getenv( (const char *) "XAPP_DESCRIPTOR_PATH" )) == NULL ) { - data = (const char *) "./config-file.json"; + } else { + filename = "./config-file.json"; } - return jparse( std::string( data ) ); + return jparse( filename ); } // --------------------- construction, destruction ------------------------------------------- diff --git a/test/Makefile b/test/Makefile index 2a97c69..7be7eee 100644 --- a/test/Makefile +++ b/test/Makefile @@ -3,7 +3,6 @@ coverage_opts = -ftest-coverage -fprofile-arcs binaries = unit_test jhash_test config_test metrics_test include = -I ../src/xapp -I ../src/alarm -I ../src/messaging -I ../src/config -I ../ext/jsmn -I ../src/json -I ../src/metrics -ld_path = LD_LIBRARY_PATH=../src/.build tests:: $(binaries) diff --git a/test/config_test.cpp b/test/config_test.cpp index 68506c1..6cf87cd 100644 --- a/test/config_test.cpp +++ b/test/config_test.cpp @@ -83,7 +83,7 @@ int main( int argc, char** argv ) { errors += fail_if( c == NULL, "unable to allocate a config with alternate name" ); auto s = c->Get_control_str( "ves_collector_address" ); - errors += fail_if( s.empty(), "expected control string not found" ); + errors += fail_if( s.empty(), "expected collector address control string not found" ); fprintf( stderr, " collector address string var: %s\n", s.c_str() ); s = c->Get_port( "rmr-data-out" ); @@ -105,26 +105,6 @@ int main( int argc, char** argv ) { auto b = c->Get_control_bool( "debug_mode" ); errors += fail_if( b == false, "epxected debug mode control boolean not found or had wrong value" ); - - // ----- test sussing path and using default name ---------------------------------- - fprintf( stderr, " sussing info from default (no name)\n" ); - c = new xapp::Config( ); // drive for coverage - - fprintf( stderr, " sussing info from default (env var == ./config1.json)\n" ); - setenv( (char *) "XAPP_DESCRIPTOR_PATH", "./config1.json", 1 ); // this var name is bad; it's not a path, but fname - c = new xapp::Config( ); - - s = c->Get_control_str( "ves_collector_address" ); - errors += fail_if( s.empty(), "expected collector address control string not found" ); - fprintf( stderr, " string var: %s\n", s.c_str() ); - - v = c->Get_control_value( "measurement_interval" ); - errors += fail_if( v == 0.0, "expected measurement interval control value not found" ); - - b = c->Get_control_bool( "debug_mode" ); - errors += fail_if( b == false, "expected debug mode control boolean not found" ); - - auto cs = c->Get_contents(); if( fail_if( cs.empty(), "get contents returned an empty string" ) == 0 ) { fprintf( stderr, " contents from file: %s\n", cs.c_str() ); @@ -134,6 +114,28 @@ int main( int argc, char** argv ) { } + // ----- test sussing path and using default name ---------------------------------- + fprintf( stderr, " sussing info from default (no env XAPP_DESCRIPTOR_PATH)\n" ); + unsetenv( "XAPP_DESCRIPTOR_PATH" ); + c = new xapp::Config( ); // no env XAPP_DESCRIPTOR_PATH; assume the config-file.json is in the same directory + errors += fail_if( c->Get_contents().empty(), "no env XAPP_DESCRIPTOR_PATH : expected default config-file.json not found" ); + + fprintf( stderr, " sussing info using XAPP_DESCRIPTOR_PATH as a directory without trailing slash\n" ); + setenv( (char *) "XAPP_DESCRIPTOR_PATH", ".", 1 ); // this env var is a directory path + c = new xapp::Config( ); + errors += fail_if( c->Get_contents().empty(), "env XAPP_DESCRIPTOR_PATH=. : expected default config-file.json not found" ); + + fprintf( stderr, " sussing info using XAPP_DESCRIPTOR_PATH as a directory with trailing slash\n" ); + setenv( (char *) "XAPP_DESCRIPTOR_PATH", "./", 1 ); // this env var is a directory path with trailing slash + c = new xapp::Config( ); + errors += fail_if( c->Get_contents().empty(), "env XAPP_DESCRIPTOR_PATH=./ : expected default config-file.json not found" ); + + fprintf( stderr, " sussing info from XAPP_DESCRIPTOR_PATH as filename\n" ); + setenv( (char *) "XAPP_DESCRIPTOR_PATH", "./config1.json", 1 ); // this var name is ok; it's an fname + c = new xapp::Config( ); + errors += fail_if( c->Get_contents().empty(), "XAPP_DESCRIPTOR_PATH as fname : expected config1.json not found" ); + + // -------------- force callback to drive and test --------------------------------- fprintf( stderr, " load config-file.json for listener coverage testing\n" ); -- 2.16.6