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 <alexandrehuff@utfpr.edu.br>
Change-Id: I1d29c8c8ae80028accc5d3e29ea571b6365121ac

diff --git a/CHANGES b/CHANGES
index 5cc1d83..5d6ee6e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -5,6 +5,10 @@
 # has several changes.  Multiple blank lines between versions are
 # squished to one.
 
 # 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.
 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 (file)
--- 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( 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" )
 
 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 (file)
--- a/src/config/config.cpp
+++ b/src/config/config.cpp
@@ -44,6 +44,7 @@
 #include <sys/inotify.h>
 #include <unistd.h>
 #include <string.h>
 #include <sys/inotify.h>
 #include <unistd.h>
 #include <string.h>

+#include <sys/stat.h>

 
 #include <iostream>
 #include <sstream>
 
 #include <iostream>
 #include <sstream>


@@ -111,7 +112,7 @@ void xapp::Config::Listener( ) {
                        if( errno == EAGAIN ) {
                                continue;
                        } else {
                        if( errno == EAGAIN ) {
                                continue;
                        } else {

-                               fprintf( stderr, "<XFCPP ### CRIT ### config listener read err: %s\n", strerror( errno ) );
+                               fprintf( stderr, "<XFCPP> ### CRIT ### config listener read err: %s\n", strerror( errno ) );

                                return;
                        }
                }
                                return;
                        }
                }


@@ -143,6 +144,10 @@ std::shared_ptr<xapp::Jhash> xapp::Config::jparse( std::string ufname ) {
        fname = ufname;
 
        std::ifstream ifs( fname );
        fname = ufname;
 
        std::ifstream ifs( fname );

+       if( ! ifs.is_open() ) {
+               fprintf( stderr, "<XFCPP> ### WARN ### unable to open %s; %s\n", fname.c_str(), strerror( errno ) );
+       }
+

        std::string st( (std::istreambuf_iterator<char>( ifs ) ), (std::istreambuf_iterator<char>() ) );
 
        auto new_jh = std::shared_ptr<xapp::Jhash>( new xapp::Jhash( st.c_str() ) );
        std::string st( (std::istreambuf_iterator<char>( ifs ) ), (std::istreambuf_iterator<char>() ) );
 
        auto new_jh = std::shared_ptr<xapp::Jhash>( new xapp::Jhash( st.c_str() ) );


@@ -156,18 +161,30 @@ std::shared_ptr<xapp::Jhash> 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
 
        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::Jhash> xapp::Config::jparse( ) {
        const char*     data;
 */
 std::shared_ptr<xapp::Jhash> 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, "<XFCPP> ### 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 -------------------------------------------
 }
 
 // --------------------- construction, destruction -------------------------------------------

diff --git a/test/Makefile b/test/Makefile
index 2a97c69..7be7eee 100644 (file)
--- 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
 
 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)
 
 
 tests::        $(binaries)
 

diff --git a/test/config_test.cpp b/test/config_test.cpp
index 68506c1..6cf87cd 100644 (file)
--- 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( 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, "<INFO> collector address string var: %s\n", s.c_str() );
 
        s = c->Get_port( "rmr-data-out" );
        fprintf( stderr, "<INFO> 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" );
 
        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, "<INFO> sussing info from default (no name)\n" );
-       c = new xapp::Config(  );                                                       // drive for coverage
-
-       fprintf( stderr, "<INFO> 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, "<INFO> 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, "<INFO> contents from file: %s\n", cs.c_str() );
        auto cs = c->Get_contents();
        if( fail_if( cs.empty(), "get contents returned an empty string" ) == 0 ) {
                fprintf( stderr, "<INFO> 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, "<INFO> 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, "<INFO> 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, "<INFO> 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, "<INFO> 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, "<INFO> load config-file.json for listener coverage testing\n" );
        // -------------- force callback to drive and test ---------------------------------
 
        fprintf( stderr, "<INFO> load config-file.json for listener coverage testing\n" );