Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cleanup a number of issues. #2763

Merged
merged 3 commits into from
Oct 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,11 @@ install-data-hook:
all-local: liblib/libnetcdf.la
echo ${PACKAGE_VERSION} > VERSION
if ENABLE_S3_TESTALL
rm -f ${abs_top_builddir}/tmp_@[email protected]
echo "@TESTUID@" >> ${abs_top_builddir}/s3cleanup_@[email protected]
cat ${abs_top_builddir}/s3cleanup_@[email protected] | sort | uniq > ${abs_top_builddir}/tmp_@[email protected]
rm -f ${abs_top_builddir}/s3cleanup_@[email protected]
mv ${abs_top_builddir}/tmp_@[email protected] ${abs_top_builddir}/s3cleanup_@[email protected]
endif

if ENABLE_S3_TESTALL
Expand Down
4 changes: 2 additions & 2 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ Release Notes {#RELEASE_NOTES}
This file contains a high-level description of this package's evolution. Releases are in reverse chronological order (most recent first). Note that, as of netcdf 4.2, the `netcdf-c++` and `netcdf-fortran` libraries have been separated into their own libraries.

## 4.9.3 - TBD

* Mitigate the problem of remote/nczarr-related test interference. See [Github #2755](https://github.com/Unidata/netcdf-c/pull/2755).
* Cleanup a number of misc issues. See [Github #2763](https://github.com/Unidata/netcdf-c/pull/2763).
* Mitigate the problem of test interference. See [Github #2755](https://github.com/Unidata/netcdf-c/pull/2755).
* Extend NCZarr to support unlimited dimensions. See [Github #2755](https://github.com/Unidata/netcdf-c/pull/2755).
* Fix significant bug in the NCZarr cache management. See [Github #2737](https://github.com/Unidata/netcdf-c/pull/2737).
* Fix default parameters for caching of NCZarr. See [Github #2734](https://github.com/Unidata/netcdf-c/pull/2734).
Expand Down
5 changes: 4 additions & 1 deletion include/ncrc.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ typedef struct NCRCinfo {
NClist* s3profiles; /* NClist<struct AWSprofile*> */
} NCRCinfo;

/* Opaque structures */
struct NCS3INFO;

#if defined(__cplusplus)
extern "C" {
#endif
Expand Down Expand Up @@ -94,7 +97,7 @@ EXTERNL int NC_getactives3profile(NCURI* uri, const char** profilep);
EXTERNL int NC_s3profilelookup(const char* profile, const char* key, const char** valuep);
EXTERNL int NC_authgets3profile(const char* profile, struct AWSprofile** profilep);
EXTERNL int NC_iss3(NCURI* uri);
EXTERNL int NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** newurlp);
EXTERNL int NC_s3urlrebuild(NCURI* url, struct NCS3INFO* s3, NCURI** newurlp);

#if defined(__cplusplus)
}
Expand Down
6 changes: 6 additions & 0 deletions include/ncs3sdk.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,19 @@
#ifndef NCS3SDK_H
#define NCS3SDK_H 1

/* Track the server type, if known */
typedef enum NCS3SVC {NCS3UNK=0, /* unknown */
NCS3=1, /* s3.amazon.aws */
NCS3GS=0 /* storage.googleapis.com */
} NCS3SVC;

typedef struct NCS3INFO {
char* host; /* non-null if other*/
char* region; /* region */
char* bucket; /* bucket name */
char* rootkey;
char* profile;
NCS3SVC svc;
} NCS3INFO;

#ifdef __cplusplus
Expand Down
4 changes: 3 additions & 1 deletion libdispatch/dinfermodel.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ static const struct MACRODEF {
{"xarray","mode",{"zarr", NULL}},
{"noxarray","mode",{"nczarr", "noxarray", NULL}},
{"zarr","mode",{"nczarr","zarr", NULL}},
{"gs3","mode",{"gs3","nczarr",NULL}}, /* Google S3 API */
{NULL,NULL,{NULL}}
};

Expand Down Expand Up @@ -196,6 +197,7 @@ static struct NCPROTOCOLLIST {
{"dods","http","mode=dap2"},
{"dap4","http","mode=dap4"},
{"s3","s3","mode=s3"},
{"gs3","gs3","mode=gs3"},
{NULL,NULL,NULL} /* Terminate search */
};

Expand Down Expand Up @@ -914,7 +916,7 @@ NC_infermodel(const char* path, int* omodep, int iscreate, int useparallel, void
/* If s3, then rebuild the url */
if(NC_iss3(uri)) {
NCURI* newuri = NULL;
if((stat = NC_s3urlrebuild(uri,NULL,NULL,&newuri))) goto done;
if((stat = NC_s3urlrebuild(uri,NULL,&newuri))) goto done;
ncurifree(uri);
uri = newuri;
} else if(strcmp(uri->protocol,"file")==0) {
Expand Down
10 changes: 6 additions & 4 deletions libdispatch/drc.c
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,9 @@ rccompile(const char* filepath)
NCURI* uri = NULL;
char* nextline = NULL;
NCglobalstate* globalstate = NC_getglobalstate();
char* bucket = NULL;
NCS3INFO s3;

memset(&s3,0,sizeof(s3));

if((ret=NC_readfile(filepath,tmp))) {
nclog(NCLOGWARN, "Could not open configuration file: %s",filepath);
Expand Down Expand Up @@ -484,9 +486,8 @@ rccompile(const char* filepath)
if(NC_iss3(uri)) {
NCURI* newuri = NULL;
/* Rebuild the url to S3 "path" format */
nullfree(bucket);
bucket = NULL;
if((ret = NC_s3urlrebuild(uri,&bucket,NULL,&newuri))) goto done;
NC_s3clear(&s3);
if((ret = NC_s3urlrebuild(uri,&s3,&newuri))) goto done;
ncurifree(uri);
uri = newuri;
newuri = NULL;
Expand Down Expand Up @@ -546,6 +547,7 @@ rccompile(const char* filepath)
rcorder(rc);

done:
NC_s3clear(&s3);
if(contents) free(contents);
ncurifree(uri);
ncbytesfree(tmp);
Expand Down
95 changes: 65 additions & 30 deletions libdispatch/ds3util.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#undef AWSDEBUG

#define AWSHOST ".amazonaws.com"
#define GOOGLEHOST "storage.googleapis.com"

enum URLFORMAT {UF_NONE=0, UF_VIRTUAL=1, UF_PATH=2, UF_S3=3, UF_OTHER=4};

Expand All @@ -44,15 +45,12 @@ Rebuild an S3 url into a canonical path-style url.
If region is not in the host, then use specified region
if provided, otherwise us-east-1.
@param url (in) the current url
@param region (in) region to use if needed; NULL => us-east-1
(out) region from url or the input region
@param bucketp (in) bucket to use if needed
(out) bucket from url
@param s3 (in/out) NCS3INFO structure
@param pathurlp (out) the resulting pathified url string
*/

int
NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** newurlp)
NC_s3urlrebuild(NCURI* url, NCS3INFO* s3, NCURI** newurlp)
{
int i,stat = NC_NOERR;
NClist* hostsegments = NULL;
Expand All @@ -63,6 +61,7 @@ NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** ne
char* host = NULL;
char* path = NULL;
char* region = NULL;
NCS3SVC svc = NCS3UNK;

if(url == NULL)
{stat = NC_EURL; goto done;}
Expand All @@ -83,14 +82,27 @@ NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** ne
Path: https://s3.<region>.amazonaws.com/<bucket-name>/<path> (3)
or: https://s3.amazonaws.com/<bucket-name>/<path> -- region defaults to us-east-1 (4)
S3: s3://<bucket-name>/<path> (5)
Other: https://<host>/<bucket-name>/<path> (6)
Google: https://storage.googleapis.com/<bucket-name>/<path> (6)
or: gs3://<bucket-name>/<path> (7)
Other: https://<host>/<bucket-name>/<path> (8)
*/
if(url->host == NULL || strlen(url->host) == 0)
{stat = NC_EURL; goto done;}

/* Reduce the host to standard form such as s3.amazonaws.com by pulling out the
region and bucket from the host */
if(strcmp(url->protocol,"s3")==0 && nclistlength(hostsegments)==1) { /* Format (5) */
bucket = nclistremove(hostsegments,0);
/* region unknown at this point */
/* Host will be set to canonical form later */
svc = NCS3;
} else if(strcmp(url->protocol,"gs3")==0 && nclistlength(hostsegments)==1) { /* Format (7) */
bucket = nclistremove(hostsegments,0);
/* region unknown at this point */
/* Host will be set to canonical form later */
svc = NCS3GS;
} else if(endswith(url->host,AWSHOST)) { /* Virtual or path */
svc = NCS3;
/* If we find a bucket as part of the host, then remove it */
switch (nclistlength(hostsegments)) {
default: stat = NC_EURL; goto done;
Expand All @@ -99,32 +111,39 @@ NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** ne
/* bucket unknown at this point */
break;
case 4: /* Format (2) or (3) */
if(strcasecmp(nclistget(hostsegments,1),"s3")==0) { /* Format (2) */
if(strcasecmp(nclistget(hostsegments,0),"s3")!=0) { /* Presume format (2) */
/* region unknown at this point */
bucket = nclistremove(hostsegments,0); /* Note removeal */
bucket = nclistremove(hostsegments,0); /* Make canonical */
} else if(strcasecmp(nclistget(hostsegments,0),"s3")==0) { /* Format (3) */
region = strdup(nclistget(hostsegments,1));
region = nclistremove(hostsegments,1); /* Make canonical */
/* bucket unknown at this point */
} else /* ! Format (2) and ! Format (3) => error */
{stat = NC_EURL; goto done;}
break;
case 5: /* Format (1) */
if(strcasecmp(nclistget(hostsegments,1),"s3")!=0)
{stat = NC_EURL; goto done;}
region = strdup(nclistget(hostsegments,2));
bucket = strdup(nclistremove(hostsegments,0));
/* Make canonical */
region = nclistremove(hostsegments,2);
bucket = nclistremove(hostsegments,0);
break;
}
} else { /* Presume Format (6) */
} else if(strcasecmp(url->host,GOOGLEHOST)==0) { /* Google (6) */
if((host = strdup(url->host))==NULL)
{stat = NC_ENOMEM; goto done;}
/* region is unknown */
/* bucket is unknown at this point */
svc = NCS3GS;
} else { /* Presume Format (8) */
if((host = strdup(url->host))==NULL)
{stat = NC_ENOMEM; goto done;}
/* region is unknown */
/* bucket is unknown */
}

/* region = (1) from url, (2) inoutregion, (3) default */
if(region == NULL)
region = (inoutregionp?nulldup(*inoutregionp):NULL);
/* region = (1) from url, (2) s3->region, (3) default */
if(region == NULL && s3 != NULL)
region = nulldup(s3->region);
if(region == NULL) {
const char* region0 = NULL;
/* Get default region */
Expand All @@ -133,23 +152,30 @@ NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** ne
}
if(region == NULL) {stat = NC_ES3; goto done;}

/* bucket = (1) from url, (2) inoutbucket */
/* bucket = (1) from url, (2) s3->bucket */
if(bucket == NULL && nclistlength(pathsegments) > 0) {
bucket = nclistremove(pathsegments,0); /* Get from the URL path; will reinsert below */
}
if(bucket == NULL)
bucket = (inoutbucketp?nulldup(*inoutbucketp):NULL);
if(bucket == NULL && s3 != NULL)
bucket = nulldup(s3->bucket);
if(bucket == NULL) {stat = NC_ES3; goto done;}

if(host == NULL) { /* Construct the revised host */
if(svc == NCS3) {
/* Construct the revised host */
ncbytesclear(buf);
ncbytescat(buf,"s3.");
ncbytescat(buf,region);
ncbytescat(buf,AWSHOST);
nullfree(host);
host = ncbytesextract(buf);
} else if(svc == NCS3GS) {
nullfree(host);
host = strdup(GOOGLEHOST);
}

/* Construct the revised path */
ncbytesclear(buf);

/* Construct the revised path */
if(bucket != NULL) {
ncbytescat(buf,"/");
ncbytescat(buf,bucket);
Expand All @@ -159,10 +185,13 @@ NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** ne
ncbytescat(buf,nclistget(pathsegments,i));
}
path = ncbytesextract(buf);

/* complete the new url */
if((newurl=ncuriclone(url))==NULL) {stat = NC_ENOMEM; goto done;}
ncurisetprotocol(newurl,"https");
assert(host != NULL);
ncurisethost(newurl,host);
assert(path != NULL);
ncurisetpath(newurl,path);
/* Rebuild the url->url */
ncurirebuild(newurl);
Expand All @@ -171,9 +200,11 @@ NC_s3urlrebuild(NCURI* url, char** inoutbucketp, char** inoutregionp, NCURI** ne
fprintf(stderr,">>> NC_s3urlrebuild: final=%s bucket=%s region=%s\n",uri->uri,bucket,region);
#endif
if(newurlp) {*newurlp = newurl; newurl = NULL;}
if(inoutbucketp) {*inoutbucketp = bucket; bucket = NULL;}
if(inoutregionp) {*inoutregionp = region; region = NULL;}

if(s3 != NULL) {
s3->bucket = bucket; bucket = NULL;
s3->region = region; region = NULL;
s3->svc = svc;
}
done:
nullfree(region);
nullfree(bucket)
Expand Down Expand Up @@ -218,7 +249,7 @@ NC_s3urlprocess(NCURI* url, NCS3INFO* s3, NCURI** newurlp)
s3->profile = strdup(profile0);

/* Rebuild the URL to path format and get a usable region and optional bucket*/
if((stat = NC_s3urlrebuild(url,&s3->bucket,&s3->region,&url2))) goto done;
if((stat = NC_s3urlrebuild(url,s3,&url2))) goto done;
s3->host = strdup(url2->host);
/* construct the rootkey minus the leading bucket */
pathsegments = nclistnew();
Expand Down Expand Up @@ -268,7 +299,7 @@ NC_s3clear(NCS3INFO* s3)
}

/*
Check if a url has indicators that signal an S3 url.
Check if a url has indicators that signal an S3 or Google S3 url.
*/

int
Expand All @@ -277,13 +308,17 @@ NC_iss3(NCURI* uri)
int iss3 = 0;

if(uri == NULL) goto done; /* not a uri */
/* is the protocol "s3"? */
/* is the protocol "s3" or "gs3" ? */
if(strcasecmp(uri->protocol,"s3")==0) {iss3 = 1; goto done;}
/* Is "s3" in the mode list? */
if(NC_testmode(uri,"s3")) {iss3 = 1; goto done;}
if(strcasecmp(uri->protocol,"gs3")==0) {iss3 = 1; goto done;}
/* Is "s3" or "gs3" in the mode list? */
if(NC_testmode(uri,"s3")) {iss3 = 1; goto done;}
if(NC_testmode(uri,"gs3")) {iss3 = 1; goto done;}
/* Last chance; see if host looks s3'y */
if(endswith(uri->host,AWSHOST)) {iss3 = 1; goto done;}

if(uri->host != NULL) {
if(endswith(uri->host,AWSHOST)) {iss3 = 1; goto done;}
if(strcasecmp(uri->host,GOOGLEHOST)==0) {iss3 = 1; goto done;}
}
done:
return iss3;
}
Expand Down
16 changes: 11 additions & 5 deletions libdispatch/nch5s3comms.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@

/*****************/

#include "ncs3sdk.h"
#include "nch5s3comms.h" /* S3 Communications */

/****************/
Expand Down Expand Up @@ -1063,7 +1064,7 @@ NCH5_s3comms_s3r_execute(s3r_t *handle, const char* url,
*----------------------------------------------------------------------------
*/
s3r_t *
NCH5_s3comms_s3r_open(const char* root, const char *region, const char *access_id, const char* access_key)
NCH5_s3comms_s3r_open(const char* root, NCS3SVC svc, const char *region, const char *access_id, const char* access_key)
{
int ret_value = SUCCEED;
size_t tmplen = 0;
Expand Down Expand Up @@ -1092,10 +1093,15 @@ NCH5_s3comms_s3r_open(const char* root, const char *region, const char *access_i
* RECORD THE ROOT PATH
*************************************/

/* Verify that the region is a substring of root */
if(region != NULL && region[0] != '\0') {
if(strstr(root,region) == NULL)
HGOTO_ERROR(H5E_ARGS, NC_EINVAL, NULL, "region not present in root path.");
switch (svc) {
case NCS3:
/* Verify that the region is a substring of root */
if(region != NULL && region[0] != '\0') {
if(strstr(root,region) == NULL)
HGOTO_ERROR(H5E_ARGS, NC_EINVAL, NULL, "region not present in root path.");
}
break;
default: break;
}
handle->rootpath = nulldup(root);

Expand Down
2 changes: 1 addition & 1 deletion libdispatch/nch5s3comms.h
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ EXTERNL hrb_t *NCH5_s3comms_hrb_init_request(const char *resource, const char *h
* DECLARATION OF S3REQUEST ROUTINES *
*************************************/

EXTERNL s3r_t *NCH5_s3comms_s3r_open(const char* root, const char* region, const char* id, const char* access_key);
EXTERNL s3r_t *NCH5_s3comms_s3r_open(const char* root, NCS3SVC svc, const char* region, const char* id, const char* access_key);

EXTERNL int NCH5_s3comms_s3r_close(s3r_t *handle);

Expand Down
2 changes: 2 additions & 0 deletions libdispatch/nclog.c
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,11 @@ ncvlog(int level, const char* fmt, va_list ap)
const char* prefix;

if(!nclogginginitialized) ncloginit();

if(nclog_global.loglevel < level || nclog_global.nclogstream == NULL) {
return;
}

prefix = nctagname(level);
fprintf(nclog_global.nclogstream,"%s: ",prefix);
if(fmt != NULL) {
Expand Down
Loading
Loading